Change epoch of TimeUnixTAI to 1970-01-01T00:00:00.0 TAI, fixes #11245 by maxnoe · Pull Request #11249 · astropy/astropy

maxnoe · 2021-01-14T17:51:49Z

TimeUnixTAI (name unix_tai) used the epoch 1970-01-01T00:00:00 UTC
since its introduction in astropy 4.2, however, contrary to the
documented behaviour, this is not the definition of CLOCK_TAI on linux
and no system using this epoch is known.
This switches the epoch to 1970-01-01T00:00:00.0 TAI to match the
intended defintion of being compatible to CLOCK_TAI on linux and also
is the definition of the now obselete and not yet released
TimeUnixPTP, which is removed in this commit.

maxnoe · 2021-01-14T17:55:18Z

pllim · 2021-01-14T18:02:19Z

As @taldcroft mentioned in #11245, #11143 is slated for 4.3 and not yet released. However, you requested this to be milestoned to 4.2.x, so what other PR is this reverting that was released in 4.2; Can you please clarify? Thanks!

mhvk

Looks good, thanks! One thing to add is a quick change for the epoch for unix_tai listed in docs/time/index.rst (in "Time from epoch formats"; I hope I caught everything...)

mhvk · 2021-01-14T19:55:54Z

Oops, failure is real too: the format list in one of the docstrings needs to be adjusted.

maxnoe · 2021-01-14T19:59:32Z

@pllim

TL;DR

This is a bug fix for a feature introduced in 4.2 which makes an already merged feature planned for 4.3 obsolete.
This is why this is milestoned for 4.2.x but reverts a PR milestoned 4.3.

Summarizing the discussions in #11245 and #11143 the situation and how we got here is as follows:

Add TimeUnixTai (unix_tai) time format class #10081 (released with astropy 4.2) introduced the new time format TimeUnixTAI with epoch 1970-01-01T00:00:00.0 UTC.
Add TimeUnixPTP to time formats #11143 Assuming that the epoch choice was intentional and correct, another new time format TimeUnixPTP with epoch 1970-01-01T00:00:00.0 TAI, which is different by the initial offset between TAI and UTC of 8.000082 seconds is added, this is not yet released.

We now realized, that the proper epoch for TimeUnixTAI should have been 1970-01-01T00:00:00.0 TAI.
This makes UnixTimePTP unecessary, so it is removed.

r-owen

Thank you for making this change.

taldcroft · 2021-01-14T20:26:45Z

Sorry @maxnoe, you kind of beat me to it, but I was planning to literally revert #11143 by hitting the revert button on GitHub. I think that is cleaner than taking it out as part of this fix.

taldcroft · 2021-01-14T20:27:54Z

Then this fix will be nothing more than changing '1970-01-01 00:00:08' to '1970-01-01 00:00:00' in the right places and fixing tests.

taldcroft · 2021-01-14T20:31:12Z

Doing it this way makes backporting this change possible, at least if I understand things.

maxnoe · 2021-01-14T20:33:26Z

Yes I agree, will rebase.

maxnoe · 2021-01-14T20:41:17Z

Done, this now only contains the changes to TimeUnixTAI and the TimeUnixPTP PR can be reverted via github.

taldcroft · 2021-01-14T20:45:39Z

See #11251.

maxnoe · 2021-01-15T09:58:05Z

@pllim I misread the changelog. UnixTimeTAI was already part of 4.1, so we should milestone for the next 4.1.x release here.

taldcroft · 2021-01-15T11:05:38Z

 class TimeUnix(TimeFromEpoch):
    """
-    Unix time: seconds from 1970-01-01 00:00:00 UTC, ignoring leap seconds.
+    Unix time (UTC): seconds from 1970-01-01 00:00:00 UTC, subject to leap seconds.


The words subject to are ambiguous. Please leave this as it was originally.

Ignoring is just wrong. UTC is the only timescale that has leap seconds. Subject to may be ambiguous, but it is rightly so because it confers to the reader that it is not as simple as "ignoring" and that one needs to understand what is actually happening.

In this context "ignoring leap seconds" is equivalent to the definition of unix time which is computing the seconds since the epoch assuming exactly 86400 seconds in every day. The actual unix time on a computer is as simple as ignoring, no?

The value of Unix time on a computer is related to the UTC time name components (year, yday, hour, min, sec) according to the expression:

tm_sec + tm_min*60 + tm_hour*3600 + tm_yday*86 400 + (tm_year-70)*31 536 000 + ((tm_year-69)/4*86 400

taldcroft · 2021-01-15T11:10:39Z


    For example, 946684800.0 in Unix time is midnight on January 1, 2000.

+    In unix time, each UTC day always has exactly 86400 seconds, even on


Please keep PR's in scope. I think that the new content is actually confusing the situation because of the mix here between POSIX unix time and astropy unix_time. In particular with the new text, the meaning of "NOTE: this quantity .." becomes ambiguous. I think the original paragraph is sufficient, so please just revert these new paragraphs.

taldcroft · 2021-01-15T11:33:14Z

+    and the linux CLOCK_TAI (https://www.cl.cam.ac.uk/~mgk25/posix-clocks.html).
+
+    For more information on this time format definition, see:
+    https://www-acc.gsi.de/wiki/Timing/TimingSystemButisInterface#Time


I'm not sure this link is relevant in this context. The "time format definition" from the typical user perspective is quite simple and stated clearly in the first line. The link provides a lot of information on a complex protocol which uses TAI, but I didn't learn anything more about the time format definition.

This was copied from the PTP docs that were introduced and now reverted. Since the two formats are now the same I thought it should be merged.

I was thinking of PTP as the format introduced in the context of these physics libraries you reference, and unix_tai as the one for most astronomers. On re-reviewing this as a change to the unix_tai docs it just seems out of place and not a generally useful link for most users of the format.

taldcroft · 2021-01-15T11:40:07Z

      >>> jyear = np.linspace(2000, 2001, 20)
      >>> t = Time(jyear, format='jyear', scale='utc')
-      >>> plt.plot_date(t.plot_date, jyear)
+      >>> plt.plot_date(t.plot_date, jyear) # doctest: +IGNORE_OUTPUT


Why was this needed?

because plot_date returns a Line object which lead to a doctest failure.

That makes sense, but makes me wonder why it was passing tests before. Did something else change in the meantime that made this doctest line suddenly start failing?

@maxnoe - did this fail for you locally or in the CI tests? (good to change regardless, but also good to understand indeed why this did not trigger problems before!)

It failed locally, did not check the CI, I fixed this together with other test failures so I don't know whether this also affected CI or whether something with my local setup was strange. Can change back to check.

Actually, right now we instruct the test to be skipped:

astropy/astropy/time/formats.py

Line 35 in ef40111

__doctest_skip__ = ['TimePlotDate']

If with you IGNORE_OUTPUT, this works, perhaps we can change it to a matplotlib requirement?

__doctest_requires__ = {'TimePlotDate': ['matplotlib']}

(Though if you prefer to just leave things as they were, that's fine too!)

I think this is best for another PR, will revert the change here.

maxnoe · 2021-01-18T16:51:21Z

@taldcroft I removed most of the docs changes and will open another PR about that.

We should get this fixed, so I don't want to discuss about leap second wordings here.

taldcroft

Just one last doc suggestion, otherwise good. Thanks @maxnoe !

…py#11245 * TimeUnixTAI (name unix_tai) used the epoch ``1970-01-01T00:00:00 UTC`` since its introduction in astropy 4.1, however, contrary to the documented behaviour, this is not the definition of CLOCK_TAI on linux and no system using this epoch is known. * This switches the epoch to ``1970-01-01T00:00:00.0 TAI`` to match the intended defintion of being compatible to CLOCK_TAI on linux and also is the definition of the now obselete and already removed ``TimeUnixPTP``, which was thus never part of a realeased version of astropy.

taldcroft

Thanks @maxnoe !

taldcroft · 2021-01-22T11:22:19Z

ci/circleci: image-tests-mpldev failure looks unrelated so merging this.

Change epoch of TimeUnixTAI to 1970-01-01T00:00:00.0 TAI, fixes #11245

github-actions Bot added the time label Jan 14, 2021

pllim added this to the v4.2.1 milestone Jan 14, 2021

pllim added the Bug label Jan 14, 2021

pllim requested review from mhvk and taldcroft January 14, 2021 17:57

mhvk requested changes Jan 14, 2021

View reviewed changes

Comment thread astropy/time/formats.py Resolved

r-owen reviewed Jan 14, 2021

View reviewed changes

Comment thread astropy/time/formats.py Resolved

maxnoe force-pushed the fix_unix_tai branch from 15a35a5 to a01a0ab Compare January 14, 2021 20:40

maxnoe mentioned this pull request Jan 15, 2021

Revert "Merge pull request #11143 from maxnoe/unix_time_ptp" #11251

Merged

maxnoe force-pushed the fix_unix_tai branch from a01a0ab to fdc226f Compare January 15, 2021 10:57

taldcroft requested changes Jan 15, 2021

View reviewed changes

pllim mentioned this pull request Jan 15, 2021

The unix_tai time format appears to be off by 8 seconds #11245

Closed

maxnoe force-pushed the fix_unix_tai branch 4 times, most recently from 1784555 to 0d886ca Compare January 18, 2021 16:49

taldcroft requested changes Jan 21, 2021

View reviewed changes

Comment thread astropy/time/formats.py Resolved

maxnoe force-pushed the fix_unix_tai branch from 0d886ca to eee7129 Compare January 21, 2021 16:32

taldcroft reviewed Jan 21, 2021

View reviewed changes

Comment thread astropy/time/formats.py Resolved

maxnoe force-pushed the fix_unix_tai branch from eee7129 to df7eb17 Compare January 22, 2021 10:24

taldcroft approved these changes Jan 22, 2021

View reviewed changes

taldcroft merged commit fc941c3 into astropy:master Jan 22, 2021

maxnoe deleted the fix_unix_tai branch January 22, 2021 12:06

eteq pushed a commit that referenced this pull request Mar 25, 2021

Merge pull request #11249 from maxnoe/fix_unix_tai

e739f42

Change epoch of TimeUnixTAI to 1970-01-01T00:00:00.0 TAI, fixes #11245

xbreak mentioned this pull request Apr 6, 2026

Correct unix_tai epoch documentation #19530

Merged

1 task


		For example, 946684800.0 in Unix time is midnight on January 1, 2000.

		In unix time, each UTC day always has exactly 86400 seconds, even on

Conversation

maxnoe commented Jan 14, 2021

Uh oh!

maxnoe commented Jan 14, 2021

Uh oh!

pllim commented Jan 14, 2021

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mhvk commented Jan 14, 2021

Uh oh!

maxnoe commented Jan 14, 2021

TL;DR

Uh oh!

r-owen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

taldcroft commented Jan 14, 2021

Uh oh!

taldcroft commented Jan 14, 2021

Uh oh!

taldcroft commented Jan 14, 2021

Uh oh!

maxnoe commented Jan 14, 2021

Uh oh!

maxnoe commented Jan 14, 2021

Uh oh!

taldcroft commented Jan 14, 2021

Uh oh!

maxnoe commented Jan 15, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxnoe Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maxnoe commented Jan 18, 2021

Uh oh!

taldcroft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

taldcroft left a comment

Choose a reason for hiding this comment

Uh oh!

taldcroft commented Jan 22, 2021

Uh oh!

Reviewers

maxnoe Jan 15, 2021 •

edited

Loading