-
Notifications
You must be signed in to change notification settings - Fork 7
Updated Tempo2 gps corrections to be compatible with recent versions of tempo2 #24
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
base: main
Are you sure you want to change the base?
Conversation
This means replacing gps_c0 and gps_c0p with gps_unso and gpst and adding the correction from gps. the old gps2utc.clk file should be deleted from any clock repos.
|
I can't merge this, but I'll look anyway. Probably we don't need to include the updated log files in the PR? |
|
Shouldn't this PR get merged? I guess it needs some conflicts fixed and the log files removed. |
|
OK, I've read this a bit more closely and I do have a concern. Tempo2 no longer has any |
|
I think PINT actually uses a file that is constructed by the clock repo itself out of BIPM data: |
|
That looks like the same comment as the new No, they are slightly different, but the same idea I guess. |
|
So that's good. PINT doesn't use the Tempo2 repo to get So we just need to merge this in, along with #29 and #31 and we should be in better shape. Might still need some more edits to the update script if we want the new files introduced in this PR to get updated automatically? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't delete this file. It is generated from BIPM and is used by PINT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a problem though - if we leave this file then it will get into tempo2 and break tempo2. If pint just uses the tempo2 clock files, why won't it just use the new clock chain instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, this is an interesting problem. We should discuss this at a PINT telecon @dlakaplan . I guess you are saying that if we leave this file, and Tempo2 uses this repo for its clock corrections (which I think is a great idea to have as many codes as possible use this centralized repo of clock corrections), then Tempo2 might get this gps2utc.clk, rather than the other clock correction chain you prefer, right? I thought maybe Tempo2 had a way to specify the preferred clock correction chain for a particular situation? So, why doesn't PINT just do the clock correction chain that Tempo2 does? I'm not sure PINT can, currently. It has a much simpler clock correction logic than Tempo2 does and doesn't normally follow a path through multiple clock correction files to get from one time system to another. I think something like this was partially implemented for Nancay, but I don't know the specifics. This is worth looking into. We'd certainly need to add some tests to the CI to make sure it ends up doing what we want.
paulray
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would love to get this merged, once the conflicts are fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main branch here has gpst2utc.clk copied from Tempo2 bitbucket. See if that is what you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will have conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. There has also been several other changes since this was originally posted since e.g. the files from BIPM also changed. I have a version on my computer that I think "works", at least for tempo2, but I think I've now paralell fixed bugs that are also separately fixed in the main branch so all a bit complicated.
Perhaps best to try to make a new PR that merges together all the changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure a new PR based on the current main branch of this repo sounds like a good plan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like the bipm.py changes might be separable, so maybe make a simple PR just for that?
|
@SixByNine I think all the instances of UNSO in your comment PR comment, and in the file name gps_unso2utc.clk should really be USNO (for United States Naval Observatory), right? |
The GPS correction in tempo2 was a bit strange, and recent (since about start of 2024) tempo2 has used a new system derived from this repository, but made compatible with tempo2.
gps2utc.clk used to be a correction that did not have a consistent definition of GPS time. This is because BIPM published two different corrections between their realisation of UTC and GPS, labeled as C0 and C0' in CircularT. Furthermore, gps2utc.clk was produced from old circularT files that had an error that was later corrected by BIPM.
The difference is that there is GPS time as directly read from the satellites (which I call GPST) and a time derived from this which better approximates UTC(UNSO), which I call (GPS(UNSO)).
Previously the IPTA clock repo tried to recreate the gps2utc.clk by switching between C0 and C0' at an arbitrary epoch. However, this is not extremely satisfactory.
Instead tempo2 has abandoned gps2utc.clk and instead we have two clock files:
gpst2utc.clk (which used C0 to go from GPST to UTC) and gps_unso2utc.clk (which uses C0' to go from GPS(UNSO) to UTC)
Note here, UTC means UTC as defined by BIPM, because that's what tempo2 implies right now.
For backwards compatibility there are two additional clock files, gps2gpst.clk and gps2gps_unso.clk, which have zero correction and fixed date ranges that basically replicate the old gps2utc behavior.
Ideally clock correction files from obervatories will change to specify which GPS time they really mean. Likely this is GPS(UNSO), but it may depend on exactly what is measured by the GPS reciever.
There is also a minor 'compatibility' fix - bipm.py previously produced duplicated entries a the start and end of circularT blocks. I think this can cause problems in tempo2, so I have updated it to avoid duplicated date entries.
If this change is merged, then tempo2 and the IPTA clock repo should agree and we can make tempo2 copy the files in from this repo (currently they are generated by a modified version of this code), and eliminate one duplication.