bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

$ emacs -Q
M-x calendar RET
M

This will show the following phases of the moon:

Saturday, January 7, 2023: Full Moon 12:07am (CET) 
Sunday, January 15, 2023: Last Quarter Moon 3:17am (CET) **  Eclipse possible **
Saturday, January 21, 2023: New Moon 9:56pm (CET) 
Saturday, January 28, 2023: First Quarter Moon 4:20pm (CET) **  Eclipse **
Sunday, February 5, 2023: Full Moon 7:28pm (CET) 
Monday, February 13, 2023: Last Quarter Moon 5:07pm (CET) **  Eclipse possible **
[...]

Note that it shows eclipses for first and last quarter moon which is
astronomically impossible.


In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.17.6) of 2023-02-09 built on localhost
Repository revision: 1518fc5d7c5bedbbe35053696c7ec06020c81b05
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101005
System Description: Gentoo Linux

Configured using:
 'configure --prefix=/usr --build=x86_64-pc-linux-gnu
 --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
 --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
 --localstatedir=/var/lib --datarootdir=/usr/share
 --disable-silent-rules --docdir=/usr/share/doc/emacs-30.0.9999
 --htmldir=/usr/share/doc/emacs-30.0.9999/html --libdir=/usr/lib64
 --program-suffix=-emacs-30-vcs --includedir=/usr/include/emacs-30-vcs
 --infodir=/usr/share/info/emacs-30-vcs --localstatedir=/var
 --enable-locallisppath=/etc/emacs:/usr/share/emacs/site-lisp
 --without-compress-install --without-hesiod --without-pop
 --with-file-notification=inotify --with-pdumper --enable-acl
 --with-dbus --with-modules --with-gameuser=:gamestat --with-libgmp
 --with-gpm --without-native-compilation --without-json
 --without-kerberos --without-kerberos5 --with-lcms2 --with-xml2
 --without-mailutils --without-selinux --without-sqlite3 --with-gnutls
 --without-libsystemd --with-threads --without-tree-sitter
 --without-wide-int --with-sound=alsa --with-zlib --with-x
 --without-pgtk --without-ns --without-gconf --with-gsettings
 --without-toolkit-scroll-bars --with-xpm --with-xft --with-cairo
 --with-harfbuzz --with-libotf --with-m17n-flt --with-x-toolkit=lucid
 --with-xaw3d --with-gif --with-jpeg --with-png --with-rsvg --with-tiff
 --without-webp --with-imagemagick --with-dumping=pdumper
 'CFLAGS=-march=native -ggdb -O2 -pipe' 'LDFLAGS=-Wl,-O1
 -Wl,--as-needed''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ
IMAGEMAGICK JPEG LCMS2 LIBOTF LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF X11 XAW3D XDBE XIM XINPUT2
XPM LUCID ZLIB

Important settings:
  value of $LC_CTYPE: en_GB.UTF-8
  value of $LC_TIME: en_GB.UTF-8
  value of $LANG: POSIX
  locale-coding-system: utf-8-unix

Major mode: Special

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils time-date subr-x
cl-loaddefs cl-lib cal-julian lunar solar cal-dst mule-util cal-move
cal-menu calendar cal-loaddefs rmc iso-transl tooltip cconv eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo x-toolkit
xinput2 x multi-tty make-network-process emacs)

Memory information:
((conses 16 45225 9891)
 (symbols 48 5661 0)
 (strings 32 15659 2104)
 (string-bytes 1 452825)
 (vectors 16 10193)
 (vector-slots 8 157214 14550)
 (floats 8 507 444)
 (intervals 56 611 0)
 (buffers 976 14))

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Müller]

This patch for lunar.el fixes the problem for me:

From cde66af556a76beb4c4232c4056fe8b60dbafe30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Sun, 12 Feb 2023 20:57:49 +0100
Subject: [PATCH] Fix spurious display of eclipses in Calendar

* lisp/calendar/lunar.el (eclipse-check): Don't show an eclipse
unless it's new moon or full moon. (bug#61460)
---
 lisp/calendar/lunar.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/calendar/lunar.el b/lisp/calendar/lunar.el
index 0db811417af..70681f42c90 100644
--- a/lisp/calendar/lunar.el
+++ b/lisp/calendar/lunar.el
@@ -161,7 +161,9 @@ remainder mod 4 gives the phase: 0 new moon, 1 first quarter, 2 full moon,
          (phase-name (cond ((= phase 0) "Solar")
                            ((= phase 2) "Lunar")
                            (t ""))))
-    (cond ((< moon-lat 2.42600766e-1)
+    (cond ((string= phase-name "")
+	   "")
+	  ((< moon-lat 2.42600766e-1)
 	   (concat "** " phase-name " Eclipse **"))
 	  ((< moon-lat 0.37)
 	   (concat "** " phase-name " Eclipse possible **"))
-- 
2.39.1

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Eli Zaretskii]

> From: Ulrich Müller <ulm@gentoo.org>
> Date: Sun, 12 Feb 2023 21:07:09 +0100
> 
> This patch for lunar.el fixes the problem for me:
> 
> >From cde66af556a76beb4c4232c4056fe8b60dbafe30 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
> Date: Sun, 12 Feb 2023 20:57:49 +0100
> Subject: [PATCH] Fix spurious display of eclipses in Calendar
> 
> * lisp/calendar/lunar.el (eclipse-check): Don't show an eclipse
> unless it's new moon or full moon. (bug#61460)

Thanks, but what about

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20414#14

It sounds like the "impossible" argument was already voiced in that
discussion?

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

>>>>> On Sun, 12 Feb 2023, Eli Zaretskii wrote:

> Thanks, but what about

>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20414#14

> It sounds like the "impossible" argument was already voiced in that
> discussion?

Indeed, and it was ignored. :(

An eclipse can only occur when the three bodies (Sun, Earth, and Moon)
are in a straight line configuration. Which is the case for new moon and
full moon, but not for quarter moon.

See also https://eclipse.gsfc.nasa.gov/LEdecade/LEdecade2021.html which
lists 2023-05-05 and 2023-10-28 as the only dates of lunar eclipses in
2023, but _not_ 2023-01-28. That output of Calendar is definitely wrong.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Eli Zaretskii]

> From: Ulrich Mueller <ulm@gentoo.org>
> Cc: 61460@debbugs.gnu.org
> Date: Sun, 12 Feb 2023 22:13:49 +0100
> 
> >>>>> On Sun, 12 Feb 2023, Eli Zaretskii wrote:
> 
> > Thanks, but what about
> 
> >   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20414#14
> 
> > It sounds like the "impossible" argument was already voiced in that
> > discussion?
> 
> Indeed, and it was ignored. :(
> 
> An eclipse can only occur when the three bodies (Sun, Earth, and Moon)
> are in a straight line configuration. Which is the case for new moon and
> full moon, but not for quarter moon.
> 
> See also https://eclipse.gsfc.nasa.gov/LEdecade/LEdecade2021.html which
> lists 2023-05-05 and 2023-10-28 as the only dates of lunar eclipses in
> 2023, but _not_ 2023-01-28. That output of Calendar is definitely wrong.

Fine, then please install this on the emacs-29 branch (assuming all
the tests still pass after the change), and thanks.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Ulrich Müller <ulm@gentoo.org> writes:

> @@ -161,7 +161,9 @@ remainder mod 4 gives the phase: 0 new moon, 1 first quarter, 2 full moon,
>           (phase-name (cond ((= phase 0) "Solar")
>                             ((= phase 2) "Lunar")
>                             (t ""))))
> -    (cond ((< moon-lat 2.42600766e-1)
> +    (cond ((string= phase-name "")
> +	   "")
> +	  ((< moon-lat 2.42600766e-1)
>  	   (concat "** " phase-name " Eclipse **"))
>  	  ((< moon-lat 0.37)
>  	   (concat "** " phase-name " Eclipse possible **"))

Sorry if I am misunderstanding, but is this good enough?  Then I don't
understand.  This doesn't go specifically to you only.

What I understand is: there are two conditions that have to be met at
the same time, and these are more or less independent over time: (1) the
latitude of the moon has to be smaller than a certain angle, and (2) it
has to be new moon or full moon.  Correct?

My questions:

(1) AFAIU, the "phase name" is derived from one of four values of the
moon "phase".  Is this really good enough to decide whether it is new
moon or full moon?  AFAIU the four moon phases all have the same length.
So AFAIU the test you added is not strong enough, we must first test
whether it is full moon or new moon, only at these days can an eclipse
happen, and only for these exact dates we need to check the moon's
latitude for whether it is small enough for an eclipse.

(2) https://en.wikipedia.org/wiki/Lunar_node tells that the limit for
the longitude of the moon is different for lunar vs. solar eclipses.
The same will be the case when we test the latitude.  The test we
currently use doesn't reflect that.  Should it?


Thanks,

Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> >           (phase-name (cond ((= phase 0) "Solar")
> >                             ((= phase 2) "Lunar")
> >                             (t ""))))
> > -    (cond ((< moon-lat 2.42600766e-1)
> > +    (cond ((string= phase-name "")
> > +	   "")
> > +	  ((< moon-lat 2.42600766e-1)
> >  	   (concat "** " phase-name " Eclipse **"))
> >  	  ((< moon-lat 0.37)
> >  	   (concat "** " phase-name " Eclipse possible **"))
>

> (1) AFAIU, the "phase name" is derived from one of four values of the
> moon "phase".  Is this really good enough to decide whether it is new
> moon or full moon?

I understand this now: That code is only ever called at the day of the
start of each (quarter) moon phase, so your patch should be correct, in
my opinion.


> (2) https://en.wikipedia.org/wiki/Lunar_node tells that the limit for
> the longitude of the moon is different for lunar vs. solar eclipses.
> The same will be the case when we test the latitude.  The test we
> currently use doesn't reflect that.  Should it?

I still have that question, though.  I only found values for the
longitude, we test the latitude (which is available in the surrounding
code, this should be no problem).  But it really looks to me like those
two values 0.24 and 0.37 should depend on the moon phase (or kind of
eclipse respectively) instead of testing both.


Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> My questions: [...]

BTW (3), I also don't understand those conversions of the latitude:

#+begin_src emacs-lisp
(defun eclipse-check (moon-lat phase)
  (let* ((moon-lat (* (/ float-pi 180) moon-lat))
         (moon-lat (abs (- moon-lat (* (floor (/ moon-lat float-pi))
                                       float-pi))))
         (moon-lat (if (> moon-lat 0.37)
                       (- float-pi moon-lat)
                     moon-lat))
         (...))
    (...)))
#+end_src

What does this do?  Don't we just want to convert a value in [0 360) to
one in [-pi pi] and use the absolute value of that, or so?  That would
look like

  (abs (* (/ float-pi 180) (- moon-lat 180)))

Why is our calculation so complicated?


Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Why is our calculation so complicated?

Why the is latitude of the moon:

#+begin_src emacs-lisp
(moon-lat (mod
            (+ 21.2964
               (* 390.67050646 index)
               (* -0.0016528 time time)
               (* -0.00000239 time time time))
            360.0))
#+end_src

calculated as a mod 360.0 value?  I would expect this for the longitude,
latitude should be in -90°...90° AFAIU.  Is it a typo?

Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

>>>>> On Mon, 13 Feb 2023, Michael Heerdegen wrote:

> BTW (3), I also don't understand those conversions of the latitude:

> #+begin_src emacs-lisp
> (defun eclipse-check (moon-lat phase)
>   (let* ((moon-lat (* (/ float-pi 180) moon-lat))
>          (moon-lat (abs (- moon-lat (* (floor (/ moon-lat float-pi))
>                                        float-pi))))
>          (moon-lat (if (> moon-lat 0.37)
>                        (- float-pi moon-lat)
>                      moon-lat))
>          (...))
>     (...)))
> #+end_src

> What does this do?  Don't we just want to convert a value in [0 360) to
> one in [-pi pi] and use the absolute value of that, or so?  That would
> look like

>   (abs (* (/ float-pi 180) (- moon-lat 180)))

> Why is our calculation so complicated?

IIUC, the goal is to calculate the angular distance from the ascending
or descending node, whichever is closer. So one wants this:

     0° ->  0°
    10° -> 10°
   ...
   170° -> 10°
   180° ->  0°
   190° -> 10°
   ...
   350° -> 10°

The only thing that I don't understand is the constant 0.37. I would
have expected pi/2 (= 90°) there. Probably it doesn't matter, because
below it checks for (< moon-lat 0.37), so any larger value will be
ignored.

>>>>> On Mon, 13 Feb 2023, Michael Heerdegen wrote:

> Why the is latitude of the moon:

> #+begin_src emacs-lisp
> (moon-lat (mod
>             (+ 21.2964
>                (* 390.67050646 index)
>                (* -0.0016528 time time)
>                (* -0.00000239 time time time))
>             360.0))
> #+end_src

> calculated as a mod 360.0 value?  I would expect this for the longitude,
> latitude should be in -90°...90° AFAIU.  Is it a typo?

No, the argument of latitude is the angle of the body measured from
the ascending node of its orbit. So its values are expected to be
between 0° and 360°.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Ulrich Mueller <ulm@gentoo.org> writes:

> IIUC, the goal is to calculate the angular distance from the ascending
> or descending node, whichever is closer. So one wants this:
>
>      0° ->  0°
>     10° -> 10°
>    ...
>    170° -> 10°
>    180° ->  0°
>    190° -> 10°
>    ...
>    350° -> 10°

Ok, thanks.  So the input value is the longitude?

> The only thing that I don't understand is the constant 0.37. I would
> have expected pi/2 (= 90°) there. Probably it doesn't matter, because
> below it checks for (< moon-lat 0.37), so any larger value will be
> ignored.

Maybe you gained slightly faster code on a really old computer - the
book that is mentioned in the code is quite old.  In theory this allows
you to get rid of one `if' statement (the comparison against 0.37 is
performed anyway).  And yes, it doesn't affect the result.

Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Ulrich Mueller <ulm@gentoo.org> writes:
>
> > IIUC, the goal is to calculate the angular distance from the ascending
> > or descending node, whichever is closer. So one wants this:
> >
> >      0° ->  0°
> >     10° -> 10°
> >    ...
> >    170° -> 10°
> >    180° ->  0°
> >    190° -> 10°
> >    ...
> >    350° -> 10°
>
> Ok, thanks.  So the input value is the longitude?

Which (unless I'm misunderstanding in what coordinate system we are
here) would be bad, since the longitude of the lunar nodes is not
constant (the nodes travel along the ecliptic in only 18.6 years)...?

Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

>>>>> On Mon, 13 Feb 2023, Michael Heerdegen wrote:

>> Ok, thanks.  So the input value is the longitude?

> Which (unless I'm misunderstanding in what coordinate system we are
> here) would be bad, since the longitude of the lunar nodes is not
> constant (the nodes travel along the ecliptic in only 18.6 years)...?

No, it is the "argument of latitude" (and yes, the nomenclature is
slighly confusing). Its value is 0° for the ascending node, which is
what we want.

https://en.wikipedia.org/wiki/Argument_of_latitude
| It is the sum of the more commonly used true anomaly and argument
| of periapsis.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Ulrich Mueller <ulm@gentoo.org> writes:

> No, it is the "argument of latitude" (and yes, the nomenclature is
> slighly confusing). Its value is 0° for the ascending node, which is
> what we want.
>
> https://en.wikipedia.org/wiki/Argument_of_latitude
> | It is the sum of the more commonly used true anomaly and argument
> | of periapsis.

Thank you very much.  So "argument" is like the argument of a complex
number, and this one correlates with the latitude, or something like
that.  Hmm, I think adding some small comments to the code would not
harm, you seem to understand it.

Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Eli Zaretskii]

> Cc: 61460@debbugs.gnu.org
> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Mon, 13 Feb 2023 11:04:46 +0100
> 
> Ulrich Mueller <ulm@gentoo.org> writes:
> 
> > No, it is the "argument of latitude" (and yes, the nomenclature is
> > slighly confusing). Its value is 0° for the ascending node, which is
> > what we want.
> >
> > https://en.wikipedia.org/wiki/Argument_of_latitude
> > | It is the sum of the more commonly used true anomaly and argument
> > | of periapsis.
> 
> Thank you very much.  So "argument" is like the argument of a complex
> number, and this one correlates with the latitude, or something like
> that.  Hmm, I think adding some small comments to the code would not
> harm, you seem to understand it.

Yes, please add such comments there, and thanks.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

>> Thank you very much.  So "argument" is like the argument of a complex
>> number, and this one correlates with the latitude, or something like
>> that.

I wondered about the term too, but it appears to be very old. It can
be found with its modern definition already in "The Equatorie of the
Planetis" from 1393 [1]: "Þe argument of latitude is þe distaunce of
þe mone from þe hed of his dragoun þat is in þe ecliptik lyne."
(The "head of the dragon" is the ascending node.)

[1] https://www.google.de/books/edition/The_Authorship_of_the_Equatorie_of_the_P/eG3JBxlp0LEC?hl=de&gbpv=1&dq=%22argument%20of%20latitude%22&pg=PA235&printsec=frontcover

>> Hmm, I think adding some small comments to the code would not harm,
>> you seem to understand it.

> Yes, please add such comments there, and thanks.

This would require that I understand the code. :) I happen to have a
copy of Dershowitz & Reingold "Calendrical Calculations" (3rd ed.).
The calculations of the lunar orbit there (on pages 193-207) are well
commented, but seem to be very different from those in lunar.el
(for example, they contain corrections for the gravitational pull of
Venus and Jupiter).

So, I'll see what I can do, but please don't expect anything soon.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Eli Zaretskii]

> From: Ulrich Mueller <ulm@gentoo.org>
> Cc: Michael Heerdegen <michael_heerdegen@web.de>,  61460@debbugs.gnu.org
> Date: Mon, 13 Feb 2023 14:30:59 +0100
> 
> > Yes, please add such comments there, and thanks.
> 
> This would require that I understand the code. :) I happen to have a
> copy of Dershowitz & Reingold "Calendrical Calculations" (3rd ed.).
> The calculations of the lunar orbit there (on pages 193-207) are well
> commented, but seem to be very different from those in lunar.el
> (for example, they contain corrections for the gravitational pull of
> Venus and Jupiter).
> 
> So, I'll see what I can do, but please don't expect anything soon.

The stuff you explained to Michael can already produce a few valuable
comments, I think.  Of course, more comprehensive comments will be
welcome.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Ulrich Mueller <ulm@gentoo.org> writes:

> https://en.wikipedia.org/wiki/Argument_of_latitude
> | It is the sum of the more commonly used true anomaly and argument
> | of periapsis.

I did not find good discussions of the eclipse limits in the English
Wikipedia version; I found good explanations in German however:

  https://de.wikipedia.org/wiki/Finsternis-Limit#Finsternis-Limite_bei_Sonnenfinsternissen

  https://de.wikipedia.org/wiki/Finsterniszyklus

AFAIU you can indeed use similar values for lunar vs. solar eclipses.
But in the case of the moon, we are then including penumbral lunar
eclipses - in the total version see

  https://en.wikipedia.org/wiki/Total_penumbral_lunar_eclipse

which are probably not really interesting (moon gets only a bit darker).

The limits for an eclipse to happen are not really constant, but taking
this into account is probably out of scope of the currently available
code in lunar.el.

Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

>>>>> On Tue, 14 Feb 2023, Michael Heerdegen wrote:
> I did not find good discussions of the eclipse limits in the English
> Wikipedia version; I found good explanations in German however:

>   https://de.wikipedia.org/wiki/Finsternis-Limit#Finsternis-Limite_bei_Sonnenfinsternissen

Thanks. I followed the legend of the diagram and was pointed to the
following page, which contains a more comprehensive explanation:
https://www.swetzel.ch/astronomie/finster/finster.html#3

> The limits for an eclipse to happen are not really constant, but taking
> this into account is probably out of scope of the currently available
> code in lunar.el.

I wonder why the angles in above explanation are completely different
from the ones given in eclipse-check:

	  ((< moon-lat 2.42600766e-1)
	   (concat "** " phase-name " Eclipse **"))
	  ((< moon-lat 0.37)
	   (concat "** " phase-name " Eclipse possible **"))

These are in radians. The first angle is 13.9° (exactly) and the
second one is 21.2°. They are in argument of latitude, while (IIUC)
the angles in "Finsternis-Limit" are given in ecliptic longitude.
Still, with the moon's orbital inclination being only 5°, one wouldn't
expect much of a difference there. Or am I missing something?

I've also found the book mentioned in lunar.el:
https://books.google.de/books?id=vVBPtkABpUoC&lpg=PA186&vq=eclipse&hl=de&pg=PA177#v=onepage&q&f=false

CCing Nicholas Strauss (who had originally submitted the code in
bug #20414).

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Ulrich Mueller <ulm@gentoo.org> writes:

> I wonder why the angles in above explanation are completely different
> from the ones given in eclipse-check:
>
> 	  ((< moon-lat 2.42600766e-1)
> 	   (concat "** " phase-name " Eclipse **"))
> 	  ((< moon-lat 0.37)
> 	   (concat "** " phase-name " Eclipse possible **"))
>
> These are in radians. The first angle is 13.9° (exactly) and the
> second one is 21.2°. They are in argument of latitude, while (IIUC)
> the angles in "Finsternis-Limit" are given in ecliptic longitude.
> Still, with the moon's orbital inclination being only 5°, one wouldn't
> expect much of a difference there. Or am I missing something?

Indeed, I wondered too about those numbers (also a bit about the format
and the number of digits) and don't have an explanation.


AFAIU (rectangular spherical triangle) we should have something like

 tan 𝚫u = tan 𝚫λ / cos i  [did I get it right?]

(with u: argument of latitude, λ: ecliptic longitude, 𝚫 meaning distance
from lunar node, i inclination (~ 5 degrees)).  The conversion does not
make much of a difference.


Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Müller]

Next round: The following simplifies the calculation and IMHO makes it
much easier to understand. Results are identical, and tests pass.

Alternatively, we could leave the result in degrees (as all other
calculations in lunar.el are in degrees) and change the values in the
final cond to 13.9 and 21.2 degrees. This would also get rid of the
excessive number of digits.


From 32609db770004aa787c05753a1f90fe906c07c60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Tue, 14 Feb 2023 11:12:29 +0100
Subject: [PATCH] ; Simplify eclipse calculation in calendar/lunar.el

* lisp/calendar/lunar.el (eclipse-check): Calculate moon-lat in
degrees, then convert to radians.
---
 lisp/calendar/lunar.el | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/lisp/calendar/lunar.el b/lisp/calendar/lunar.el
index 8ced4144105..ec31ea596ea 100644
--- a/lisp/calendar/lunar.el
+++ b/lisp/calendar/lunar.el
@@ -155,14 +155,11 @@ remainder mod 4 gives the phase: 0 new moon, 1 first quarter, 2 full moon,
 ;; from "Astronomy with your Personal Computer", Subroutine Eclipse
 ;; Line 7000 Peter Duffett-Smith Cambridge University Press 1990
 (defun eclipse-check (moon-lat phase)
-  (let* ((moon-lat (* (/ float-pi 180) moon-lat))
-         ;; For positions near the ascending or descending node,
-         ;; calculate the absolute angular distance from that node.
-         (moon-lat (abs (- moon-lat (* (floor (/ moon-lat float-pi))
-                                       float-pi))))
-         (moon-lat (if (> moon-lat 0.37) ; FIXME (* 0.5 float-pi)
-                       (- float-pi moon-lat)
-                     moon-lat))
+  (let* ((moon-lat (mod moon-lat 180))
+         ;; Calculate the absolute angular distance from the ascending
+         ;; or descending node, whichever is nearer.
+         (moon-lat (min moon-lat (- 180 moon-lat)))
+         (moon-lat (degrees-to-radians moon-lat))
          (phase-name (cond ((= phase 0) "Solar")
                            ((= phase 2) "Lunar")
                            (t ""))))
-- 
2.39.1

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Ulrich Müller <ulm@gentoo.org> writes:

> Next round: The following simplifies the calculation and IMHO makes it
> much easier to understand. Results are identical, and tests pass.

Yes, thanks.  I want to suggest to get rid of the successive rebinding
to the variable `moon-lat' and use a different name (`node-distance'
maybe?) since that calculation is not just a simple conversion of units.

> Alternatively, we could leave the result in degrees (as all other
> calculations in lunar.el are in degrees) and change the values in the
> final cond to 13.9 and 21.2 degrees. This would also get rid of the
> excessive number of digits.

Good idea.  The conversion to radians is a bit artificial and not really
useful, just harder to read.

Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Müller]

>>>>> On Tue, 14 Feb 2023, Michael Heerdegen wrote:

>> Next round: The following simplifies the calculation and IMHO makes
>> it much easier to understand. Results are identical, and tests pass.

> Yes, thanks.  I want to suggest to get rid of the successive rebinding
> to the variable `moon-lat' and use a different name (`node-distance'
> maybe?) since that calculation is not just a simple conversion of units.

>> Alternatively, we could leave the result in degrees (as all other
>> calculations in lunar.el are in degrees) and change the values in the
>> final cond to 13.9 and 21.2 degrees. This would also get rid of the
>> excessive number of digits.

> Good idea.  The conversion to radians is a bit artificial and not really
> useful, just harder to read.

Pushed to master, including your suggestion (I went for "node-dist").


I think the last open question is what values we should use as limits
for "certain" and "possible" eclipses? A calculation of these limits can
be found here (complete text on Google Books):

  William Chauvenet, "A manual of spherical and practical astronomy",
  Vol. I, Lippincott & Co., London, 1863.

On page 439: "a solar eclipse is certain if at new moon β < 1°23'15",
impossible if β > 1°34'53", and doubtful between these limits."

And on page 543: "a lunar eclipse is certain if at full moon β < 52'4",
impossible if β > 63'53", and doubtful between these limits."

These limits are given in terms of ecliptic latitude β, which can be
converted to the argument of latitude u using the same spherical
triangle as above (in message #68):

  sin Δu = sin β / sin i

Using the minimum and maximum values of the lunar orbit's inclination
i_min = 4°57'2" and i_max = 5°20'6" (see table on page 438), I get the
following limits for Δu:

  Solar eclipse certain:  Δu < 15.10°
  Solar eclipse possible: Δu < 18.65°
  Lunar eclipse certain:  Δu <  9.37°
  Solar eclipse possible: Δu < 12.43°

(These are close to the values given by Peter Duffet-Smith, "Practical
astronomy with your calculator", 3rd ed., Cambridge University Press,
1991, page 156: Solar eclipse certain: 15°31', possible: 18°31'; lunar
eclipse certain: 9°30', possible: 12°15'.)

However, if I put these limits into the comparisons in eclipse-check,
then it misses the partial lunar eclipse on 2023-10-28 (I haven't
checked any others, but if it is wrong in this year, then it is probably
wrong in more cases). I suspect that the calculations in lunar-phase are
not precise enough.

Also, I believe that I have found the source of the current limits:

  Jean Meeus, "Astronomical algorithms", 1st ed., Willmann-Bell,
  Richmond, VA, 1991.

This happens to be one of the references mentioned in the header of
lunar.el, therefore the limits are likely to be consistent with the
precision of the other calculations.

On page 350 it says: "If F [the argument of latitude] differs from the
nearest multiple of 180° by less than 13°.9, then there is certainly an
eclipse; if the difference is larger than 21°.0, there is no eclipse;
between these two values, the eclipse is uncertain at this stage and the
case must be examined further."

tl;dr I suggest that we leave the values as-is, except for a small
adjustment from 21.2 to 21.0, as in the patch included below. If there
aren't any nobjections, I'm going to push this to master.

@Michael: Can this bug be closed then?


From 431c60084dade55acd0d407fff1cdfb2352a3d91 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Thu, 16 Feb 2023 20:09:22 +0100
Subject: [PATCH] ; * lisp/calendar/lunar.el (eclipse-check): Adjust upper
 limit.

---
 lisp/calendar/lunar.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/calendar/lunar.el b/lisp/calendar/lunar.el
index 1f827ca34b0..8fb5c89c057 100644
--- a/lisp/calendar/lunar.el
+++ b/lisp/calendar/lunar.el
@@ -164,8 +164,9 @@ remainder mod 4 gives the phase: 0 new moon, 1 first quarter, 2 full moon,
                            (t ""))))
     (cond
      ((string= phase-name "") "")
+     ;; Limits 13.9° and 21.0° from Meeus (1991), page 350.
      ((< node-dist 13.9) (concat "** " phase-name " Eclipse **"))
-     ((< node-dist 21.2) (concat "** " phase-name " Eclipse possible **"))
+     ((< node-dist 21.0) (concat "** " phase-name " Eclipse possible **"))
      (t ""))))
 
 (defconst lunar-cycles-per-year 12.3685 ; 365.25/29.530588853
-- 
2.39.2

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

Ulrich Müller <ulm@gentoo.org> writes:

> Pushed to master, including your suggestion (I went for "node-dist").

Thank you very much.

> [...]
> tl;dr I suggest that we leave the values as-is, except for a small
> adjustment from 21.2 to 21.0, as in the patch included below. If there
> aren't any nobjections, I'm going to push this to master.

Fine by me.


> @Michael: Can this bug be closed then?

I have some more cosmetic changes:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lun.diff --]
[-- Type: text/x-diff, Size: 1934 bytes --]

diff --git a/lisp/calendar/lunar.el b/lisp/calendar/lunar.el
index 1f827ca34b0..ebf9abc9d60 100644
--- a/lisp/calendar/lunar.el
+++ b/lisp/calendar/lunar.el
@@ -94,7 +94,7 @@ lunar-phase
                        (* -0.0016528 time time)
                        (* -0.00000239 time time time))
                     360.0))
-	 (eclipse (eclipse-check moon-lat phase))
+	 (eclipse (lunar-check-for-eclipse moon-lat phase))
          (adjustment
           (if (memq phase '(0 2))
               (+ (* (- 0.1734 (* 0.000393 time))
@@ -154,18 +154,18 @@ lunar-phase

 ;; from "Astronomy with your Personal Computer", Subroutine Eclipse
 ;; Line 7000 Peter Duffett-Smith Cambridge University Press 1990
-(defun eclipse-check (moon-lat phase)
-  (let* ((node-dist (mod moon-lat 180))
-         ;; Absolute angular distance from the ascending or descending
-         ;; node, whichever is nearer.
-         (node-dist (min node-dist (- 180 node-dist)))
-         (phase-name (cond ((= phase 0) "Solar")
-                           ((= phase 2) "Lunar")
-                           (t ""))))
+(defun lunar-check-for-eclipse (moon-lat phase)
+  (let ((type (cond ((= phase 0) "Solar")
+                    ((= phase 2) "Lunar")
+                    (t nil)))
+        ;; Absolute angular distance from the ascending or descending
+        ;; node, whichever is nearer.
+        (node-dist (funcall (lambda (x) (min x (- 180 x)))
+                            (mod moon-lat 180))))
     (cond
-     ((string= phase-name "") "")
-     ((< node-dist 13.9) (concat "** " phase-name " Eclipse **"))
-     ((< node-dist 21.2) (concat "** " phase-name " Eclipse possible **"))
+     ((not type) "")
+     ((< node-dist 13.9) (concat "** " type " Eclipse **"))
+     ((< node-dist 21.2) (concat "** " type " Eclipse possible **"))
      (t ""))))

 (defconst lunar-cycles-per-year 12.3685 ; 365.25/29.530588853

[-- Attachment #3: Type: text/plain, Size: 115 bytes --]


In particular, the name of the global function should start with
"lunar-".  Does that look ok?


Thanks,

Michael

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Müller]

>>>>> On Fri, 17 Feb 2023, Michael Heerdegen wrote:

> I have some more cosmetic changes:

> [...]

>         (node-dist (funcall (lambda (x) (min x (- 180 x)))
>                             (mod moon-lat 180))))

I didn't go for this one, because IMHO it is too clever and would
worsen readability of the code. (Also, there is precedent for
successive rebinding in function lunar-phase.)

> In particular, the name of the global function should start with
> "lunar-".  Does that look ok?

Thank you. Updated patch below.


From 4a3abeac5c6614c9b0b346767627f978b2dd138a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Thu, 16 Feb 2023 20:09:22 +0100
Subject: [PATCH] ; Adjust limit for eclipse in calendar/lunar.el, rename
 function

* lisp/calendar/lunar.el (lunar-check-for-eclipse): Renamed from
'eclipse-check'; thanks to Michael Heerdegen for the suggestion.
Slightly adjust the upper limit for the distance from the node to
the value found in literature. (bug#61460)
---
 lisp/calendar/lunar.el | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/lisp/calendar/lunar.el b/lisp/calendar/lunar.el
index 1f827ca34b0..fe8129ec511 100644
--- a/lisp/calendar/lunar.el
+++ b/lisp/calendar/lunar.el
@@ -94,7 +94,7 @@ remainder mod 4 gives the phase: 0 new moon, 1 first quarter, 2 full moon,
                        (* -0.0016528 time time)
                        (* -0.00000239 time time time))
                     360.0))
-	 (eclipse (eclipse-check moon-lat phase))
+         (eclipse (lunar-check-for-eclipse moon-lat phase))
          (adjustment
           (if (memq phase '(0 2))
               (+ (* (- 0.1734 (* 0.000393 time))
@@ -154,19 +154,18 @@ remainder mod 4 gives the phase: 0 new moon, 1 first quarter, 2 full moon,
 
 ;; from "Astronomy with your Personal Computer", Subroutine Eclipse
 ;; Line 7000 Peter Duffett-Smith Cambridge University Press 1990
-(defun eclipse-check (moon-lat phase)
+(defun lunar-check-for-eclipse (moon-lat phase)
   (let* ((node-dist (mod moon-lat 180))
          ;; Absolute angular distance from the ascending or descending
          ;; node, whichever is nearer.
          (node-dist (min node-dist (- 180 node-dist)))
-         (phase-name (cond ((= phase 0) "Solar")
-                           ((= phase 2) "Lunar")
-                           (t ""))))
-    (cond
-     ((string= phase-name "") "")
-     ((< node-dist 13.9) (concat "** " phase-name " Eclipse **"))
-     ((< node-dist 21.2) (concat "** " phase-name " Eclipse possible **"))
-     (t ""))))
+         (type (cond ((= phase 0) "Solar")
+                     ((= phase 2) "Lunar"))))
+    (cond ((not type) "")
+          ;; Limits 13.9° and 21.0° from Meeus (1991), page 350.
+          ((< node-dist 13.9) (concat "** " type " Eclipse **"))
+          ((< node-dist 21.0) (concat "** " type " Eclipse possible **"))
+          (t ""))))
 
 (defconst lunar-cycles-per-year 12.3685 ; 365.25/29.530588853
   "Mean number of lunar cycles per 365.25 day year.")
-- 
2.39.2

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Ulrich Müller <ulm@gentoo.org> writes:

> Thank you. Updated patch below.

Looks good to me, thanks,

Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Looks good to me, thanks,
>
> Michael.

I think we are now done and can close this report.

Thank you for your work.  This one was very interesting!

Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

>>>>> On Sat, 18 Feb 2023, Michael Heerdegen wrote:

> Thank you for your work.  This one was very interesting!

Yeah, this one was fun. :)

I was also tempted to add a defcustom for the eclipse names, so we could
have "Sonnenfinsternis", etc. Then again, the additional word "possible"
in these messages made me feel that we're in l10n/gettext territory, and
presumably it should be solved in a more general context (there was a
discussion on emacs-devel in 2021).

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Ulrich Mueller <ulm@gentoo.org> writes:

> I was also tempted to add a defcustom for the eclipse names, so we could
> have "Sonnenfinsternis", etc. Then again, the additional word "possible"
> in these messages made me feel that we're in l10n/gettext territory, and
> presumably it should be solved in a more general context (there was a
> discussion on emacs-devel in 2021).

Yes.

That remark made me think about whether we want a `diary-eclipses' - or
teach `diary-lunar-phases' to report eclipses (at the moment the latter
doesn't report eclipses, I just tried).

Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

[-- Attachment #1: Type: text/plain, Size: 288 bytes --]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> That remark made me think about whether we want a `diary-eclipses' - or
> teach `diary-lunar-phases' to report eclipses (at the moment the latter
> doesn't report eclipses, I just tried).

Seems getting the latter is quite simple:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lunar-diary-eclipse.diff --]
[-- Type: text/x-diff, Size: 908 bytes --]

diff --git a/lisp/calendar/lunar.el b/lisp/calendar/lunar.el
index 4f8f34d954f..5b73bb6e29e 100644
--- a/lisp/calendar/lunar.el
+++ b/lisp/calendar/lunar.el
@@ -284,8 +284,13 @@ diary-lunar-phases
       (setq index (1+ index)
             phase (lunar-phase index)))
     (if (calendar-date-equal (car phase) date)
-        (cons mark (concat (lunar-phase-name (nth 2 phase)) " "
-                           (cadr phase))))))
+        (cons mark
+              (let ((eclipse (nth 3 phase)))
+                (concat (lunar-phase-name (nth 2 phase)) " "
+                        (cadr phase)
+                        (if (string-empty-p eclipse)
+                            ""
+                          (concat " " eclipse))))))))

 ;; For the Chinese calendar the calculations for the new moon need to be more
 ;; accurate than those above, so we use more terms in the approximation.

[-- Attachment #3: Type: text/plain, Size: 11 bytes --]



Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

>>>>> On Tue, 21 Feb 2023, Michael Heerdegen wrote:

>> That remark made me think about whether we want a `diary-eclipses' - or
>> teach `diary-lunar-phases' to report eclipses (at the moment the latter
>> doesn't report eclipses, I just tried).

> Seems getting the latter is quite simple:

> -        (cons mark (concat (lunar-phase-name (nth 2 phase)) " "
> -                           (cadr phase))))))
> +        (cons mark
> +              (let ((eclipse (nth 3 phase)))
> +                (concat (lunar-phase-name (nth 2 phase)) " "
> +                        (cadr phase)
> +                        (if (string-empty-p eclipse)
> +                            ""
> +                          (concat " " eclipse))))))))

It is probably a matter of personal taste, but I dislike the nested
concats. This seems simpler (not tested, though):

-        (cons mark (concat (lunar-phase-name (nth 2 phase)) " "
-                           (cadr phase))))))
+        (cons mark
+              (let ((eclipse (nth 3 phase)))
+                (concat (lunar-phase-name (nth 2 phase)) " "
+                        (cadr phase)
+                        (if (string-empty-p eclipse) "" " ")
+                        eclipse))))))

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Andreas Schwab]

On Feb 22 2023, Ulrich Mueller wrote:

>>>>>> On Tue, 21 Feb 2023, Michael Heerdegen wrote:
>
>>> That remark made me think about whether we want a `diary-eclipses' - or
>>> teach `diary-lunar-phases' to report eclipses (at the moment the latter
>>> doesn't report eclipses, I just tried).
>
>> Seems getting the latter is quite simple:
>
>> -        (cons mark (concat (lunar-phase-name (nth 2 phase)) " "
>> -                           (cadr phase))))))
>> +        (cons mark
>> +              (let ((eclipse (nth 3 phase)))
>> +                (concat (lunar-phase-name (nth 2 phase)) " "
>> +                        (cadr phase)
>> +                        (if (string-empty-p eclipse)
>> +                            ""
>> +                          (concat " " eclipse))))))))
>
> It is probably a matter of personal taste, but I dislike the nested
> concats. This seems simpler (not tested, though):
>
> -        (cons mark (concat (lunar-phase-name (nth 2 phase)) " "
> -                           (cadr phase))))))
> +        (cons mark
> +              (let ((eclipse (nth 3 phase)))
> +                (concat (lunar-phase-name (nth 2 phase)) " "
> +                        (cadr phase)
> +                        (if (string-empty-p eclipse) "" " ")
> +                        eclipse))))))

concat also accepts lists, so nil is pefectly fine as an argument.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Ulrich Mueller <ulm@gentoo.org> writes:

> It is probably a matter of personal taste, but I dislike the nested
> concats. This seems simpler (not tested, though):
>
> -        (cons mark (concat (lunar-phase-name (nth 2 phase)) " "
> -                           (cadr phase))))))
> +        (cons mark
> +              (let ((eclipse (nth 3 phase)))
> +                (concat (lunar-phase-name (nth 2 phase)) " "
> +                        (cadr phase)
> +                        (if (string-empty-p eclipse) "" " ")
> +                        eclipse))))))

Fine by me (my preference would be Andreas' suggestion).

We also need to fix the space handling in calendar-lunar-phases aka M in
calendar - when no eclipse occurs, the descriptions end with a
trailing space.

Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

>>>>> On Wed, 22 Feb 2023, Michael Heerdegen wrote:

> Ulrich Mueller <ulm@gentoo.org> writes:
>> It is probably a matter of personal taste, but I dislike the nested
>> concats. This seems simpler (not tested, though):
>> 
>> -        (cons mark (concat (lunar-phase-name (nth 2 phase)) " "
>> -                           (cadr phase))))))
>> +        (cons mark
>> +              (let ((eclipse (nth 3 phase)))
>> +                (concat (lunar-phase-name (nth 2 phase)) " "
>> +                        (cadr phase)
>> +                        (if (string-empty-p eclipse) "" " ")
>> +                        eclipse))))))

> Fine by me (my preference would be Andreas' suggestion).

Use something like ‘(unless (string-empty-p eclipse) " ")’? WFM.

> We also need to fix the space handling in calendar-lunar-phases aka M in
> calendar - when no eclipse occurs, the descriptions end with a
> trailing space.

While at it, maybe replace ‘(car (last x))’ by ‘(nth 3 x)’?

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

Ulrich Mueller <ulm@gentoo.org> writes:

> Use something like ‘(unless (string-empty-p eclipse) " ")’? WFM.
>
> > We also need to fix the space handling in calendar-lunar-phases aka M in
> > calendar - when no eclipse occurs, the descriptions end with a
> > trailing space.
>
> While at it, maybe replace ‘(car (last x))’ by ‘(nth 3 x)’?

I tried to do that (and a bit more):


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-also-diary-lunar-phases-report-eclipses.patch --]
[-- Type: text/x-diff, Size: 2069 bytes --]

From f0eaa97bbc5e4a6145295a4da14606012737820e Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Wed, 22 Feb 2023 14:56:07 +0100
Subject: [PATCH] Make also 'diary-lunar-phases' report eclipses

* lisp/calendar/lunar.el (diary-lunar-phases): Report eclipses.
(calendar-lunar-phases): Tweak.
---
 lisp/calendar/lunar.el | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/lisp/calendar/lunar.el b/lisp/calendar/lunar.el
index 4f8f34d954f..b395e08c232 100644
--- a/lisp/calendar/lunar.el
+++ b/lisp/calendar/lunar.el
@@ -245,10 +245,11 @@ calendar-lunar-phases
         (insert
          (mapconcat
           (lambda (x)
-            (format "%s: %s %s %s" (calendar-date-string (car x))
-                    (lunar-phase-name (nth 2 x))
-                    (cadr x)
-		    (car (last x))))
+            (let ((eclipse (nth 3 x)))
+              (concat (calendar-date-string (car x)) ": "
+                      (lunar-phase-name (nth 2 x)) " "
+                      (cadr x) (unless (string-empty-p eclipse) " ")
+		      eclipse)))
           (lunar-phase-list m1 y1) "\n")))
       (message "Computing phases of the moon...done"))))

@@ -283,9 +284,13 @@ diary-lunar-phases
     (while (calendar-date-compare phase (list date))
       (setq index (1+ index)
             phase (lunar-phase index)))
-    (if (calendar-date-equal (car phase) date)
-        (cons mark (concat (lunar-phase-name (nth 2 phase)) " "
-                           (cadr phase))))))
+    (and (calendar-date-equal (car phase) date)
+         (cons mark
+               (let ((eclipse (nth 3 phase)))
+                 (concat (lunar-phase-name (nth 2 phase)) " "
+                         (cadr phase)
+                         (unless (string-empty-p eclipse) " ")
+                         eclipse))))))

 ;; For the Chinese calendar the calculations for the new moon need to be more
 ;; accurate than those above, so we use more terms in the approximation.
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 10 bytes --]


Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

>>>>> On Wed, 22 Feb 2023, Michael Heerdegen wrote:

> +            (let ((eclipse (nth 3 x)))
> +              (concat (calendar-date-string (car x)) ": "
> +                      (lunar-phase-name (nth 2 x)) " "
> +                      (cadr x) (unless (string-empty-p eclipse) " ")
> +		      eclipse)))

Fix the indentation? The tabs in this line are the only ones in the
file.

Otherwise LGTM.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

[-- Attachment #1: Type: text/plain, Size: 482 bytes --]

Ulrich Mueller <ulm@gentoo.org> writes:

> >>>>> On Wed, 22 Feb 2023, Michael Heerdegen wrote:
>
> > +            (let ((eclipse (nth 3 x)))
> > +              (concat (calendar-date-string (car x)) ": "
> > +                      (lunar-phase-name (nth 2 x)) " "
> > +                      (cadr x) (unless (string-empty-p eclipse) " ")
> > +		      eclipse)))
>
> Fix the indentation? The tabs in this line are the only ones in the
> file.

Impressive discovery!  Updated patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-also-diary-lunar-phases-report-eclipses.patch --]
[-- Type: text/x-diff, Size: 2083 bytes --]

From 039668a5a4b27ea7e841592b9be42e50af1abe74 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Wed, 22 Feb 2023 14:56:07 +0100
Subject: [PATCH] Make also 'diary-lunar-phases' report eclipses

* lisp/calendar/lunar.el (diary-lunar-phases): Report eclipses.
(calendar-lunar-phases): Tweak.
---
 lisp/calendar/lunar.el | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/lisp/calendar/lunar.el b/lisp/calendar/lunar.el
index 4f8f34d954f..5b22043102d 100644
--- a/lisp/calendar/lunar.el
+++ b/lisp/calendar/lunar.el
@@ -245,10 +245,11 @@ calendar-lunar-phases
         (insert
          (mapconcat
           (lambda (x)
-            (format "%s: %s %s %s" (calendar-date-string (car x))
-                    (lunar-phase-name (nth 2 x))
-                    (cadr x)
-		    (car (last x))))
+            (let ((eclipse (nth 3 x)))
+              (concat (calendar-date-string (car x)) ": "
+                      (lunar-phase-name (nth 2 x)) " "
+                      (cadr x) (unless (string-empty-p eclipse) " ")
+                      eclipse)))
           (lunar-phase-list m1 y1) "\n")))
       (message "Computing phases of the moon...done"))))

@@ -283,9 +284,13 @@ diary-lunar-phases
     (while (calendar-date-compare phase (list date))
       (setq index (1+ index)
             phase (lunar-phase index)))
-    (if (calendar-date-equal (car phase) date)
-        (cons mark (concat (lunar-phase-name (nth 2 phase)) " "
-                           (cadr phase))))))
+    (and (calendar-date-equal (car phase) date)
+         (cons mark
+               (let ((eclipse (nth 3 phase)))
+                 (concat (lunar-phase-name (nth 2 phase)) " "
+                         (cadr phase)
+                         (unless (string-empty-p eclipse) " ")
+                         eclipse))))))

 ;; For the Chinese calendar the calculations for the new moon need to be more
 ;; accurate than those above, so we use more terms in the approximation.
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 11 bytes --]



Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Subject: [PATCH] Make also 'diary-lunar-phases' report eclipses
>
> * lisp/calendar/lunar.el (diary-lunar-phases): Report eclipses.
> (calendar-lunar-phases): Tweak.

I've pushed that last change (to master) now.

And: we are done, so I'm closing this report.  Thanks to everybody!


Michael.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Eli Zaretskii]

> From: Ulrich Müller <ulm@gentoo.org>
> Date: Tue, 14 Feb 2023 11:22:56 +0100
> 
> Next round: The following simplifies the calculation and IMHO makes it
> much easier to understand. Results are identical, and tests pass.

If these are just simplifications, please install on master, not on
emacs-29.

Thanks.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

>>>>> On Tue, 14 Feb 2023, Michael Heerdegen wrote:

> AFAIU (rectangular spherical triangle) we should have something like

>  tan 𝚫u = tan 𝚫λ / cos i  [did I get it right?]

> (with u: argument of latitude, λ: ecliptic longitude, 𝚫 meaning distance
> from lunar node, i inclination (~ 5 degrees)).  The conversion does not
> make much of a difference.

I think this is correct. With cos i = 0.996, taking λ or u makes
little difference.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Ulrich Mueller]

>>>>> On Mon, 13 Feb 2023, Michael Heerdegen wrote:

> Sorry if I am misunderstanding, but is this good enough?  Then I don't
> understand.  This doesn't go specifically to you only.

> What I understand is: there are two conditions that have to be met at
> the same time, and these are more or less independent over time: (1) the
> latitude of the moon has to be smaller than a certain angle, and (2) it
> has to be new moon or full moon.  Correct?

Yes.

> My questions:

> (1) AFAIU, the "phase name" is derived from one of four values of the
> moon "phase".  Is this really good enough to decide whether it is new
> moon or full moon?  AFAIU the four moon phases all have the same length.
> So AFAIU the test you added is not strong enough, we must first test
> whether it is full moon or new moon, only at these days can an eclipse
> happen, and only for these exact dates we need to check the moon's
> latitude for whether it is small enough for an eclipse.

The phase is passed to eclipse-check as an argument from lunar-phase:

| (lunar-phase INDEX)
|
| Local date and time of lunar phase INDEX.
| Integer below INDEX/4 gives the lunation number, counting from Jan 1, 1900;
| remainder mod 4 gives the phase: 0 new moon, 1 first quarter, 2 full moon,
| 3 last quarter.  Returns a list (DATE TIME PHASE).

eclipse-check is only called for the actual dates/times of the lunar
phases, and then checks for phase 0 ("Solar") or phase 2 ("Lunar").
IIUC this should guarantee that it is the exact date and time of new
moon or full moon.

> (2) https://en.wikipedia.org/wiki/Lunar_node tells that the limit for
> the longitude of the moon is different for lunar vs. solar eclipses.
> The same will be the case when we test the latitude.  The test we
> currently use doesn't reflect that.  Should it?

Probably. Also, the moon's orbit is elliptic, so one would expect the
latitude angles at perigee to be different from those at apogee.

Anyway, let's not the better be the enemy of the good. I've pushed the
patch to the emacs-29 branch (with updated tests, thanks Eli for
reminding me). Fixing the other issues would require much more work.

Are more precise prediction of eclipses within the scope of Calendar?
If yes, then this bug should be left open for a followup patch.

bug#61460: 30.0.50; Calendar shows eclipse for quarter moon

[Michael Heerdegen]

Ulrich Mueller <ulm@gentoo.org> writes:

> Are more precise prediction of eclipses within the scope of Calendar?
> If yes, then this bug should be left open for a followup patch.

If we can do better, why not.  But my concern is more whether the code
is well readable and understandable.

Michael.