Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider adding a programatic way to add leap seconds #43

Closed
eteq opened this issue Feb 9, 2017 · 38 comments
Closed

Consider adding a programatic way to add leap seconds #43

eteq opened this issue Feb 9, 2017 · 38 comments

Comments

@eteq
Copy link
Member

eteq commented Feb 9, 2017

Currently leap seconds are hard-coded into the C, as SOFA does. However, there are use cases (e.g. astropy/astropy#5783) where it would be useful to have the option for leap seconds to be added at runtime. This option should be added.

The main debate here is whether or not this is acceptable, as it's a break from how SOFA does it. I think yes, because it's a pretty important use case - users should be able to get leap seconds from online sources and update at runtime without having to recompile...

@astrofrog
Copy link

Just out of curiosity, is it a feature that has been requested upstream?

@mhvk
Copy link
Contributor

mhvk commented Feb 9, 2017

cc @scottransom for the whether or not it has been requested for SOFA not to put the leapseconds in a hard-coded part of a c file...

@scottransom
Copy link

Nope. I have not seen or heard about this request I don't think.... I could try and pass it on, though.

@mhvk
Copy link
Contributor

mhvk commented Feb 9, 2017

@scottransom - I think that would be good indeed. It does seem strange to have to reinstall or recompile a whole package just because of a new leap second.

@eteq
Copy link
Member Author

eteq commented Feb 9, 2017

@scottransom - I think we had some discussions about this years ago and got a "no", but I could be mis-remembering! So let us know what you can find out. Ideally this would be a bit of a back-and-forth if SOFA is amenable to it, though. That is, it would be good for us to have a look at any proposed solution so as to see if it works in the astropy et al. use case.

@timj
Copy link
Contributor

timj commented Feb 9, 2017

I think the main objection was one of code portability for the C parsing the text file, and, in some sense, the hard-wired numbers can never be wrong for dates earlier than the file was created (so it's important for the code to know when the table was correct). It might make sense to have a SOFA/ERFA API that lets people add new leap seconds to the internal table. Then SOFA would not have to worry about file I/O and Astropy could have its own table for loading newer leap seconds at package import. This would require the code inside SOFA to be reorganized a bit.

@eteq
Copy link
Member Author

eteq commented Feb 9, 2017

@timj - exactly what I was thinking: have a way to add leap seconds, but the ERFA/SOFA side would just be eraAddLeapSecond(jd1, jd2) or similar, and it would be left to user code when/if they wanted to do that.

@scottransom
Copy link

@timj Huh. I have to admit that I don't really see the utility of this. Maybe I'm missing something.

@eteq says in #5783 that the issue is that a bugfix release has to be made. Why is that? Why can't you simply update the ERFA code that astropy ships with, anticipating the new SOFA release? Or does changing C-code somehow mandate a bugfix release?

We know the format and way that those changes work. And we are usually given months of warning before a leap second comes in to effect....

@eteq
Copy link
Member Author

eteq commented Feb 9, 2017

@scottransom - yes, that's exactly the problem: to update the C-code we have to do a bugfix release because it's all compiled into astropy in the standard mode. The other option is for a user to use a system-installed ERFA and update ERFA itself...

But the point in astropy/astropy#5783 is that we already download the information needed to update leap seconds dynamically at runtime for other reasons. So we want to be able to make that consistent with what ERFA thinks the leap seconds are. That's good both for the "getting the latest leap second" use case and the "I want to reproduce a prediction I made years ago before the most recent leap seconds came out" case.

@scottransom
Copy link

Hmmm... OK. I'm still not sure this is enough ammunition for me to try to get SOFA changed, though.

A bigger benefit (IMO) would be that users forced to use not-up-to-date astropy's (and ERFAs) would always have correct leap seconds since they are determined dynamically by the downloaded IERS info. That would probably prevent errors. The reasons about avoiding bugfix releases seem like only (sorry!) convenience for the astropy maintainers -- and given the infrequent leap second additions, not a huge improvement in convenience at that.

Note: Am just trying to figure out how to spin this to the SOFA board

@eteq
Copy link
Member Author

eteq commented Feb 9, 2017

@scottransom - Oh, yes, sorry, the way you said it there is exactly what I see as the most important point. It's not that I so much mind issuing a bugfix, but rather that I have no faith that users will install said bugfix.

Also, from a science reproducibility standpoint I think it really is important that there's a way to specify the leap seconds... So I guess that's call to also have a eraRemoveLeapSecond and eraGetLeapSeconds or something like that... But I'm I guess I don't know whether the SOFA board considers scientific reproducibility to be a high priority?

@scottransom
Copy link

@eteq - why is reproducibility an issue? Since leap seconds aren't retroactive, they shouldn't affect anything current or past. It would only affect future predictions across leap sec boundaries. Or is that exactly what you are talking about?

@eteq
Copy link
Member Author

eteq commented Feb 9, 2017

@scottransom - Yep, exactly. For example, I write a code that tells me where some asteroid is going to be in ~1 year because I got an observing program approved or something, but then a leap second is inserted before the observations happen. I want to be able to go back and ask where I though the object was at a particular time before I knew there'd be a leap second so that I can figure out why I put that in the proposal/paper/whatever.

Almost all practical cases I work on this is a very small effect... but it might matter for e.g. VLBI or the like...? (Although I presume you would know a lot better @scottransom)

@eteq
Copy link
Member Author

eteq commented Feb 9, 2017

(I should clarify that I think the reproducibility argument is a lot less important than the other issue, though. If addLeapSecond is already being added, I'd say removeLeapSecond might as well go in at the same time for these reasons, but the former is way more important than the latter.)

@mhvk
Copy link
Contributor

mhvk commented Feb 9, 2017

I agree it is really about ensuring that people running slightly old code don't get it wrong and publish a paper on a mysterious change in, say, orbital period (has happened...). To me, easiest would be if SOFA provided a routine to access and possibly replace the whole leap second table; we can deal with inserting one at the end or removing one. But I'm not sure that is more or less of a hassle between different compilers than just putting the leap seconds in an ascii file and read it... Anyway, implementation detail...

@rmathar
Copy link

rmathar commented May 23, 2017

Leap seconds tables come in two formats:
ftp://ftp.iana.org/tz/code/leapseconds
https://www.ietf.org/timezones/data/leap-seconds.list

Reading the zic(8) man page, it seems one of these two formats is used to compile the tzfile(5) data base of Linuxes.

@PaulKuin
Copy link

PaulKuin commented May 24, 2017 via email

@scottransom
Copy link

So just to let you all know, SOFA is still discussing this and something will almost certainly change. We don't exactly know what, yet, though. Right now, I believe that the leading contender is the simplest -- just change the licensing of the leap sec file so that it may be changed as needed by the users and still compiled into the SOFA library. That would keep the default behavior for the vast majority of users, and allow others to change things as appropriate for them.

@shbhuk
Copy link

shbhuk commented Jan 15, 2018

Just wanted to check if anything has happened here. I think the user should have some warning as to the staleness of the leap second file.
The problem is that right now the leap seconds are last updated when astropy was last installed, this might be really old on an old observatory computer, and all calculations which need the latest leap second (Time conversions, etc. ) would run out of sync.
Based on the discussion in Astropy pull #6861, since adding a run time warning might be complicated or undesirable, it might be good to at least document this better in the Astropy documentation and that of the function.

This is the reason we had to write our own leap second management code for Barycorrpy, where we maintain a separate leap second file downloaded and a log file which has the date of last download. We compare this to check staleness and then download a new file, if the user has permitted updates.
https://github.com/shbhuk/barycorrpy/blob/master/barycorrpy/utc_tdb.py

astropy/astropy#5783

@olebole
Copy link
Collaborator

olebole commented Jan 15, 2018

Leap seconds are already maintained by the operating system -- for example, /usr/share/zoneinfo/leap-seconds.list on Linux. This list gets automatically updated with the OS updates; if not it would IMO be better to update this file programmatically, and to use it as the single leap second database on the system, instead of having local lists flying around in some code (possibly even statically linked, like in SAOImage DS9) in the system. IRAF has it compiled in as well (outdated; from slalib). And Casacore has its own little database for the leap seconds. Other (in source code) are in jcdf, GAVO Dachs, Starjava PAL (used by Topcat). You are happy if you use a single ecosystem only, and don't run into inconsistencies?

Please, can we -- instead of adding one more "own leap management system" -- agree on a canonical way to handle leap seconds?

@mhvk
Copy link
Contributor

mhvk commented Jan 15, 2018

Within astropy itself, we have access to updated leap seconds via IERS-A; that has the advantage of not being OS-dependent. So, at least adding the warning should be easy (PR welcome; I won't have time in the near future).

Logically, though, as a "standard of astronomy", this is handled by SOFA/ERFA... - @scottransom - any news in that regard?

@olebole - since astropy currently gets its leap seconds from SOFA, for Debian the current scheme is actually reasonably good, correct? You just have a special update for ERFA and all works?

@shbhuk
Copy link

shbhuk commented Jan 15, 2018

Hey, yes we can do that. Would be ideal to have it OS independent. IERS-A has the date of the last bulletin. However, do we not need something which has a date-wise break up of when each leap second was added?
Something, more like this - http://maia.usno.navy.mil/ser7/tai-utc.dat

@olebole
Copy link
Collaborator

olebole commented Jan 15, 2018

Not in practice: Unless I create a special update for our released Debian (and Ubuntu) versions, the ERFA package there stays at the version that way current on the Debian release day. For example, in Debian Stretch, we still have version 1.3.0. Any leap second introduced after this date (none in the moment, however) will not be there. And I am just not able to update all packages containing leap-second data for all supported platforms on a leap second announcement.
On the other hand, the /usr/share/zoneinfo/leap-seconds.list is already kept up-to-date, so it would be nice to grab the information from there. Casacore stores this in a database; there I have a small script that updates the database every time leap-seconds.list is changed. For erfa, it is however not so simple due to the required recompilation. I still find it an unclean solution to put data that is (slowly) changing directly into source code.
I still think of changing dat.c to read the leap-second.list file instead of using a compiled-in list for Debian.

@mhvk
Copy link
Contributor

mhvk commented Jan 15, 2018

@olebole - OK, that makes sense. I agree that an external list would be better for ERFA; possibly, we need to deviate from SOFA and just built that in ourselves. @eteq, @taldcroft - what do you think?

@shbhuk - one can infer the leap seconds from the IERS-A list (which we download already) quite trivially, by looking for jumps in UT1-UTC. But possibly we should just download the file you point at.

@scottransom
Copy link

Yes, we (SOFA) discussed this issue quite a bit last year. The committee didn't like the idea of changing the source code, but we are changing the license for the file so that users can modify the leap second routines as they see fit. See the 2017 Aug 25 note here http://www.iausofa.org

There is not really a great solution to this problem since sometimes users will want to do non-standard things with leap seconds (i.e. test codes without proper leap seconds in place, or make predictions for the future).

@jwoillez
Copy link
Contributor

@scottransom Would it at least be possible to make IYV, changes, and NDAT not const and global to iauDat? This would help with getting them programmatically changed. Or does no code change mean no code change at all?

Then, it would be simpler for sofa_deriver.py (https://github.com/liberfa/erfa-fetch/blob/master/sofa_deriver.py) to add a hook for this programmatic change.

@scottransom
Copy link

@jwoillez I can ask about this with the SOFA folks. Basically you would just like it if those enums became (global?) ints in dat.c?

@shbhuk
Copy link

shbhuk commented Jan 18, 2018

Is there a reason to not implement it as a stand alone TAI-UTC file, with automatic updates which one can turn off?

For the majority who want a system which updates itself, they don't have to bother. For the ones who want to play with it, they can always modify that .txt file or delete a row, as they deem fit.

@mhvk
Copy link
Contributor

mhvk commented Jan 18, 2018

@shbhuk - the text files essentially exist already - the question really is on how to allow one to use these inside erfa.

p.s. Should perhaps add in defense of the hard-coding that, generally, one better is up-to-date with the code as well - things do change beyond leap seconds. But not remotely as often...

@shbhuk
Copy link

shbhuk commented Jan 19, 2018

Which file is this?

Till we fix this issue, can we at least add a warning in astropy documentation for this in the meanwhile to create awareness?

@jwoillez
Copy link
Contributor

@scottransom: I take back what I asked. It should be OK to keep the leap seconds array changes and its size NDAT private to Dat.

All: See #49 for a proposal.

@mhvk
Copy link
Contributor

mhvk commented Nov 15, 2018

We've been silent on this for a while, but I was just talking to @luojing1211 (PINT) and one thing that became clear is that on many observatory computers, one cannot count on continuous access to the internet, so updates are done sporadically. In that sense, a fallback to a system leap second file may also be useful (if that one is not OK, everything is probably lost anyway): /usr/share/zoneinfo/leap-seconds.list (on debian at least; just a copy of the IETF file linked above). At least for linux distributions, this would also keep things coordinated, as @olebole suggested.

@eteq
Copy link
Member Author

eteq commented Nov 15, 2018

Also, should probably investigate whether #50 has changed the situation to make this all easier to do... The changelog seems to say it's just the actual "terms and conditions" not the code itself that has changed, but I could be mis-understanding (haven't looked at the diff yet).

@mhvk
Copy link
Contributor

mhvk commented Nov 15, 2018

Its only the permission that has changed, so that one can now substitute one's own file even in SOFA. For ERFA, I guess this is not as relevant (though we might as well be the one that provides a useful standard substitute).

@nikkelj
Copy link

nikkelj commented Jun 27, 2019

It is worth noting that /usr/share/zoneinfo/leap-seconds.list is an OS-dependent convention. It is good that the software packages the table, because the file might not always be available/updated. Assuming /usr/share/zoneinfo/leap-seconds.list is always available/updated is just that, an assumption.

My recommendation would be to make the software look for an environment variable (document it, of course) that indicates the location of "/usr/share/zoneinfo/leap-seconds.list", and then prefer that over the hard-coded table IF it is supplied. This would be the system independent way of solving the problem, yields great flexibility, and at least makes full automation without build updates a possibility.

@rmathar
Copy link

rmathar commented Oct 8, 2019

A function that could be called from within dat.c that searches for a leap-seconds.list file may look as follows:

 #include <string.h> 
 #include <stdlib.h>
 #include <ctype.h>
 #include <stdio.h>
 #include <time.h>
 #include <unistd.h>
 #include <sys/stat.h>

 /** Anchor file to detect a leap-seconds.list file
 */
 #define ERFA_ETC_LOCTIME "/etc/localtime"

 /** Number of seconds between unix epoch and NTP epoch. RFC 868
 */
 #define ERFA_1900_1972 2208988800L

 /**
 * @brief Scan /etc/share/zoneinfo/leap-seconds.list if that file exists.
 * @param[in] iy UTC year
 * @param[in] im UTC month, starting at Jan=1
 * @param[in] id UTC day, starting at 1.
 * @param[out] deltat leap seconds at that day.
 *    Value is only changed on output if leap-seconds file was found.
 * @return -5 if the associated leap-seconds.list file is not found.
 *            0 if at least one valid time-leap-second pair was in the file.
 * @since 2019-10-08
 * @author R. J. Mathar
*/
int eraDatTZ(int iy, int im, int id, double *deltat)
{
    int ret = -5 ;
    struct stat fstat ;
    char leapfname[256] ;

    /* typically existing on Linuxes
    */
    int status = lstat(ERFA_ETC_LOCTIME, & fstat) ;
    if ( S_ISLNK(fstat.st_mode) )
    {
        int nSize = readlink(ERFA_ETC_LOCTIME,leapfname,sizeof(leapfname)) ;
        if ( nSize > 0 )
        {
            char * zistart ;
            leapfname[nSize] = '\0' ;
            /* something like /usr/share/zoneinfo/Europe/Berlin now in leapfname
            */
            zistart = strstr(leapfname,"zoneinfo") ;
            if ( zistart != NULL)
            {
                /* delete the continent/city part of the file name,
                * and replace it by leap-seconds.txt
                */
                *(zistart+9)='\0' ;
                strncat(leapfname,"leap-seconds.list",sizeof(leapfname)-strlen(leapfname)) ;

                FILE *fd = fopen(leapfname,"r") ;
                /* this open will fail on openSUSE and CentOS but not on Ubuntu
                * or if someone added the leap-seconds.list to /usr/share/zoneinfo.
                */
                if ( fd)
                {
                    char leapline[132] ;
                    struct tm refdate ;
                    memset(&refdate,0,sizeof(refdate)) ;
                    refdate.tm_year = iy-1900 ;
                    refdate.tm_mday = id ;
                    refdate.tm_mon = im-1 ; /* month convention in mktime is Jan=0 */
                    const time_t utcref = mktime(&refdate) ;
                    while ( fgets(leapline,sizeof(leapline),fd) == leapline)
                    {
                        /* times start with a number; skip comments */
                        long long secs;
                        double leaps ;
                        int items = sscanf(leapline,"%lld %le",&secs,&leaps) ;
                        if ( items == 2)
                        {
                            /* now secs are seconds since 1900-01-01 and utcref
                            * is seconds since 1970-01-01
                            */
                            ret =0 ;
                            if ( secs -ERFA_1900_1972 > utcref)
                                break;
                            else
                                *deltat = leaps ;
                        }
                    }
                    fclose(fd) ;
                }
            }
        }
    }
    return ret ;
}

Such files apparently only exist on Ubuntu by default, not openSUSE or CentOS. On the latter systems the observer would need to copy the leap-seconds list himself into the local computer's file system.

@nikkelj
Copy link

nikkelj commented Oct 9, 2019

Nice, thanks for the starter code. Maybe we will find some time to work on this soon, the importance is going to rise for us shortly. Makes me wonder if we should open issues in openSUSE and CentOS about it too, but we could also solve the problem in our pipeline. Our software pipeline can stage the file for us on IaC-created or PaaS created hosting, and then inject an environment variable indicating its location that the software can use.

@eteq
Copy link
Member Author

eteq commented Dec 5, 2019

Sorry to have not followed-up in this thread - I wasn't watching because of the follow-on discussion in #49 . And then #58 ended up implementing the base requirement of this issue, so I'm closing this. But the last few comments above are deeply tied to #61 so I suggest we move further discussion there.

@eteq eteq closed this as completed Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests