shred / commons-suncalc

Calculate sun and moon times in Java
Apache License 2.0
127 stars 24 forks source link

SunTimes noon value very inconsistent with SunPosition azimuth #20

Closed isomeme closed 4 years ago

isomeme commented 4 years ago

The noon value from SunTimes can produce a value up to 5 degrees / 20 minutes away from when SunPosition shows sun azimuth ~= 180 at the same location. See this unit test for a demonstration; it contains a comment with more information.

isomeme commented 4 years ago

Midnight time has the same issue with variance from 0 azimuth.

shred commented 4 years ago

I've found a bug in the interpolation loop. However, the fix has changed the test results by a few seconds only, and your unit test gives an azimuth of 178.0153°, which is still far from 180°. It seems that the quadratic interpolation is not suitable for finding the peaks.

The concept of the quadratic interpolation loop was always suspect to me. Now I'm going to try a different approach. It may take a few days though, and I'm not sure if it will solve the issue. :smile:

isomeme commented 4 years ago

Cool, good luck! It sounds like you may be trying to find the minimum or maximum for solar declination. Declination has a broad peak, so it may be hard to iterate on that to zero in on the actual maximum. I'd suggest figuring out whether you're looking for az 0 or 180, and then use a simplex or whatever to iteratively get a time that takes you below some epsilon away from that.

isomeme commented 4 years ago

....And of course for that to be stable around az 0, you'd need to subtract 360 from az > 180 to get a smooth azimuth range to hunt in.

isomeme commented 4 years ago

Yep, that tripped me up at first. :) I had a wonderful time figuring out how your quadratic interpolator works -- that's amazingly cool! Then I started thinking about how to get those last few degrees of accuracy for noon and nadir given the quadratic-interpolated values of noon and nadir as starting points.

A test of my proof-of-principle solution is here. Below that line are tests showing SunTimes getting noon and nadir wrong, followed by my new NoonNadirCorrector turning them into azimuths correct within 0.25 degree. The basic idea is to assume that near noon and nadir the rate degrees/time is fairly constant. We pick a new time deltaTime from the current time, and calculate their deltaAzimuth and the error between the new azimuth and 0/180. We scale deltaTime and repeat until the error falls below 0.25 degrees (1 minute of the sun's motion).

This is sloppy code, with bad naming, System.out.printf calls, and so forth. There's a lot of room to tune things like picking the initial deltaTime more intelligently. But it seems to converge rapidly for the cases I've tested. Perhaps you can use something like this in SunTimes.

This technique wouldn't apply to sunrise and sunset, where we're not looking for a fixed degree value like 0 or 180. But it's also true that deviations in these won't be so obvious when you display the sun events on a compass graphic. :)

shred commented 4 years ago

To give proper credits: I've found the quadratic interpolator in the book "Astronomy on the Personal Computer" by Montenbruck/Pfleger. They have used it for sunrise/sunset calculation only. It was my (stupid) idea to use it for noon/nadir, too. Now I know why they didn't do it. :smile:

Thank you for your proof-of-principle! The solution to this issue is now a nested interval algorithm that locates the peaks recursively. It takes 6 steps to bring the error below 0.1 degrees. The fix changed the noon time on the Singapore and Martinique unit tests by almost two minutes!

The fixed version will be released as suncalc v3.0 soon. You will be happy to hear that suncalc v3 is using the Java Time API now, so you won't need to convert between Date and Instant objects any more.

Thank you very much for your help, and for spotting the bugs! I really appreciate it. :+1:

shred commented 4 years ago

This bug is fixed in suncalc v3.0.

Note that suncalc v2.9 only contains a partial fix! I did not port the entire fix to suncalc v2 because it uses lambda expressions, which are not available in Java 7.