Leap second handling is broken

toshimichiotsubo commented 4 years ago

Hi, I am a new user of AstroTime.jl, and I found this package very useful. I encountered an issue as below. Note that a leap second was inserted before 2017-1-1 00:00:00 UTC, but this is the case around 2016-12-31 00:00:00 UTC. The same happens for 2015-6-30.

julia> ep = UTCEpoch(2016, 12, 31, 0, 0, 0.0)
2016-12-30T23:59:59.000 UTC

julia> ep = UTCEpoch(2016, 12, 31, 0, 0, 0.1)
Error showing value of type Epoch{CoordinatedUniversalTime,Float64}:
ERROR: ArgumentError: Seconds are out of range
Stacktrace:
(followed by a long message)

julia> ep = UTCEpoch(2016, 12, 31, 0, 1, 0.0)
2016-12-31T00:00:60.000 UTC

Thanks, Toshi

helgee commented 4 years ago

That is definitely not what's supposed to happen. Thanks for reporting 👍 I will have a look as soon as possible.

helgee commented 4 years ago

Patch release is pending: https://github.com/JuliaRegistries/General/pull/17467

I had some conceptual difficulties with this issue thus it took quite a while 😬

toshimichiotsubo commented 4 years ago

Thank you very much! The above-described cases work perfect now.

However, I found (possibly new) an issue at 23:59:00 UTC as below.

julia> ep = UTCEpoch(2016, 12, 31, 23, 59, 0.0)
2016-12-31T23:58:59.000 UTC 
julia> UTCEpoch(2016, 12, 31, 23, 58, 59.0) + 1 * seconds
2016-12-31T23:59:01.000 UTC

Thanks, Toshi

helgee commented 4 years ago

I found another one:

julia> UTCEpoch(2020,3,1,23,59,60.0)
2020-03-02T00:00:00.000 UTC

helgee commented 4 years ago

Aw, snap! Houston, we may have a problem here.

I have come to the conclusion that the current data model (post #48) cannot handle leap seconds properly. Here is an example based on the latest leap second:

TAI	UTC	TAI second	Offset	UTC second
2017-01-01T00:00:35.000	2016-12-31T23:59:59.000	536500835	36	536500835 - 36 = 536500799
2017-01-01T00:00:36.000	2016-12-31T23:59:60.000	536500836	36/37	536500836 - 36 = 536500800 / 536500836 - 37 = 536500799
2017-01-01T00:00:37.000	2017-01-01T00:00:00.000	536500837	37	536500837 - 37 = 536500800
2017-01-01T00:00:38.000	2017-01-01T00:00:01.000	536500838	37	536500838 - 37 = 536500801

Due to the discontinuity in the UTC scale, the representation as UTC seconds since the J2000 epoch is ambiguous. What seems conceptually wrong about this is that we do not count all the UTC seconds, i.e. the previous leap seconds are not counted. One could do that but then all calculations for UTC epochs would need to be special cased.

Another option would be to maintain "day number" separate from "second in the day", e.g.

struct Epoch{S<:TimeScale,T}
    scale::S
    j2k_day::Int
    second::Int
    fraction::T
end

CC: @giordano @bgodard

bgodard commented 4 years ago

I would certainly not design the generic Epoch with UTC in mind. UTC is more of way of specifying time or a label for time rather than an actual timescale. It is like an alternate string representation of TAI. The specifities of UTC would make the generic epoch class complex and possibly slower. In my experience dealing with professional astrodynamics software, I have not seen anything that properly handles UTC during a leap second except in one case but this was by specializing most functions for UTC and storing UTC internally as TAI but even then this feature was not maintained and deleted since nobody cared (just avoid using UTC plus in general nobody does satellite operations during a leap second).

I really think that any UTC specifities should be dealt by specialization. I would store UTC as TAI. This would make already all UTC operations such as t2-t1, t1+delta work fine. The specializations are then needed for:

calendar/string input output
(modified) julian day input/output (although I have seen this used does UTC MJD even make sense? this would anyway be unable to encode the leap second)
function to get seconds delta between TAI and UTC (even though both epochs would have same encoding except for timescale value, this should be equal to the number of leap seconds.)

I think julia being a multiple dispatch language should make this specialization easier to implement than in almost any other languages.

One drawback I see of storing UTC as TAI is that leap seconds are needed to read/write UTC epoch but without them the UTC epoch cannot be operated properly anyway and you might as well keep UTC as a string or calendar date otherwise.

helgee commented 4 years ago

Thanks, @bgodard. Your suggestion seems like a very sensible way forward to me.

In the meantime, I have experimented with a data model as outlined above and taking some input from CCSDS 301.0-B-4. But I am not happy with the increased complexity as you predicted.

in general nobody does satellite operations during a leap second

This seems to be wise 😆

JuliaAstro / AstroTime.jl

Leap second handling is broken #50