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 astropy 4.1 in order to use the new unix_tai format #324

Closed
morcuended opened this issue Apr 3, 2020 · 10 comments
Closed

Consider astropy 4.1 in order to use the new unix_tai format #324

morcuended opened this issue Apr 3, 2020 · 10 comments

Comments

@morcuended
Copy link
Member

As discussed in #288, I suggest considering astropy 4.1 whenever it is out. It will allow us to use the newly added class that defines the unix_tai format which takes into account the leap seconds to easily transform Time objects between tai and utc scales.

In the astropy version 3.2.3 we currently use in lstchain, unix format is always considering utc scale without taking into account the leap seconds.

@rlopezcoto
Copy link
Contributor

I was testing the installation of astropy 4.0 together with @maxnoe as discussed in #333, but I found that the astropy version is hardcoded in ctapipe 0.7.0,so we cannot update astropy version until ctapipe does not get updated as well

@kosack
Copy link

kosack commented Apr 9, 2020

yes, we had to make some fixes for astropy 4.0, so it's only supported in the dev for now. We will release a new version soon, though.

@kosack
Copy link

kosack commented Apr 9, 2020

One of the big changes with astropy 4 and also the latest numpy is that now all numpy functions support astropy units (which breaks things where we had assumed units were ignored). That may cause you some problems as well. So just upgrading may not be so simple.

@rlopezcoto
Copy link
Contributor

thanks @kosack. I saw your PRs and that is why I was willing to test astropy 4 and see which errors popped up, but Travis already throws conflicts in the environment creation due to the hardcoding in ctapipe

@maxnoe
Copy link
Member

maxnoe commented Apr 9, 2020

hardcoded in ctapipe 0.7.0,

Just to clarify: it is not hardcoded. It is required to be an astropy 3.x, as we require 3 and 4.0 broke some stuff. It's not hard coded, it specifies which versions work with ctapipe.

@maxnoe
Copy link
Member

maxnoe commented Dec 7, 2020

We found out that the definitions of unix_tai in astropy 4.1 differ from the actual definition UCTS uses by 8 seconds because of using a different epoch.

UCTS uses: 1970-01-01T00:00:00 TAI = 1969-12-31T23:59:52.000 UTC

astropy unix_tai uses: 1970-01-01T00:00:08 TAI = 1969-12-31 23:59:59.999918 UTC

The ucts scale can be implemented by using:

class TimeUnixTAIUCTS(TimeUnix):
    name = 'unix_tai_ucts'
    epoch_val = '1970-01-01 00:00:00'
    epoch_scale = 'tai'

@morcuended
Copy link
Member Author

I think I also ran into this issue when playing with UCTS timestamps in UTC/TAI. I thought at that time that the 1970-01-01 00:00:00 epoch was the standard epoch and got puzzled when times were shifted by 8 seconds. I did not know what was going on back then and you implemented the tai to utc workaround function that has perfectly worked. So I guess this class has to be defined somewhere in lstchain or ctapipe_io_lst, right?

@maxnoe
Copy link
Member

maxnoe commented Dec 7, 2020

I opened a PR in astropy (ptp is a general standard, so it should apply to more people) to add it and in the mean time added it also to ctapipe_io_lst.

@maxnoe
Copy link
Member

maxnoe commented Jan 22, 2021

Ok, full circle on this: we realized that astropy just got it wrong with unix_tai in 4.1, it will be fixed in 4.2.1 and in the new version of the source.

@maxnoe
Copy link
Member

maxnoe commented Jan 17, 2022

This is now fixed.

@maxnoe maxnoe closed this as completed Jan 17, 2022
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

4 participants