Believed incorrect rise/set date computed, for regions far from UTC.

[vincentneo]

Hi again,

I now believe that issue #97 is a real issue.

If we were to look at the same chunk of code again, this time with UTC time, it will make a lot more sense why it happens:

        let formatter = DateFormatter()
        formatter.dateFormat = "yyyy-MM-dd'T'HH:mm:ssZ"
        formatter.timeZone = TimeZone(abbreviation: "UTC")
        //formatter.timeZone = TimeZone(abbreviation: "UTC+2")
                                                                       /* Location's TimeZone */    /* UTC Time */
        print(formatter.string(from: times.riseTime!.date))         // 2020-09-08T07:21:41+0200  |  2020-09-08T05:21:41+0000
        print(formatter.string(from: times.transitTime!.date))      // 2020-09-08T13:46:57+0200  |  2020-09-08T11:46:57+0000
        print(formatter.string(from: times.setTime!.date))          // 2020-09-08T20:11:16+0200  |  2020-09-08T18:11:16+0000

        //formatter.timeZone = TimeZone(abbreviation: "UTC+8")
        let singapore = GeographicCoordinates(CLLocation(latitude: 1.290, longitude: 103.852))
        let timesSG = Earth(julianDay: jd).twilights(forSunAltitude: 0, coordinates: singapore)
        print(formatter.string(from: timesSG.riseTime!.date))       // 2020-09-09T07:00:19+0800  |  2020-09-08T23:00:19+0000
        print(formatter.string(from: timesSG.transitTime!.date))    // 2020-09-08T13:01:03+0800  |  2020-09-08T05:01:03+0000
        print(formatter.string(from: timesSG.setTime!.date))        // 2020-09-08T19:01:27+0800  |  2020-09-08T11:01:27+0000

        //formatter.timeZone = TimeZone(abbreviation: "UTC-7")
        let sf = GeographicCoordinates(CLLocation(latitude: 37.7740, longitude: -122.4313))
        let timesSF = Earth(julianDay: jd).twilights(forSunAltitude: 0, coordinates: sf)
        print(formatter.string(from: timesSF.riseTime!.date))       // 2020-09-08T06:49:19-0700  |  2020-09-08T13:49:19+0000
        print(formatter.string(from: timesSF.transitTime!.date))    // 2020-09-08T13:05:58-0700  |  2020-09-08T20:05:58+0000
        print(formatter.string(from: timesSF.setTime!.date))        // 2020-09-07T19:23:31-0700  |  2020-09-08T02:23:31+0000

Let's take a look at the Singapore example. In UTC time, date is the same, but rise time is, according to UTC time, is later than set time. For Paris, it's fine, as the timezone offset is not that large to UTC time, and the time doesn't spill to the next/prev day.

The implementation of the RiseTransitSet, for rise, transit and set is as follows:

JulianDay(year: date.year,
                            month: date.month,
                            day: date.day,
                            hour: sexagesimalTransit.radical,
                            minute: sexagesimalTransit.minute,
                            second: sexagesimalTransit.second)

This meant that year, month and day is always going to be a constant date, regardless if in some regions, where the rise/set overlaps to another day or not, according to UTC time, especially in places where its timezones are of larger offsets to UTC. This should be wrong, I believe.

Which is why I have opened this pull request with a small patch that checks if rise/set date is less/more than transit time. If it is, for rise, a day will be subtracted off, while for set, a day will be added in.

[onekiloparsec]

Thanks a lot for this work. I'll go through it asap!

[onekiloparsec]

After giving a small thought, I am wondering. I agree there is an ambiguity with the date part, but should this "fix" be implemented inside SwiftAA or the "client" code ? Because I could make myself a choice here, and say, the "rise" date is the reference. But other people may prefer something else.

I agree that when providing JulianDays it would make more sense but,...

[vincentneo]

the "client" code

I was doing that prior; offsetting the date from the returned JulianDay, but it was quite error prone when I had to do it many times in many places all around the code.

Because I could make myself a choice here, and say, the "rise" date is the reference. But other people may prefer something else.

I chose transit for the comparison, because its always of the source date. I feel that it wouldn't make sense to pass in 10 Oct, and get computed results for 11 Oct's events.

[onekiloparsec]

Vincent, I realised I am using the old and deprecated RiseTransitSet class since a long time. I will close the PR, and make a release with the new class.