Improve clock file handling #1239
Conversation
|
Currently it simply reads all the clock files from the PINT repository, and I'd like to see it pull in additional or newer clock files from I think we should also handle missing corrections the same way we handle TOAs past the end of existing corrections. Which at the moment is, emit a warning but go on with uncorrected data. Longer term we should be willing to pull in updated clock files using the astropy cache system to get them from a central server, triggered by TOAs we don't have corrections for, but I don't think that belongs in this PR. |
aarchiba
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1239 +/- ##
==========================================
- Coverage 61.26% 61.02% -0.24%
==========================================
Files 88 88
Lines 19757 19895 +138
Branches 3456 3485 +29
==========================================
+ Hits 12104 12141 +37
- Misses 6897 6994 +97
- Partials 756 760 +4
Continue to review full report at Codecov.
|
|
Unclear what we should do when someone specifies |
|
I could put in a test that would fail if any of our favourite telescopes are more than (say) 90 days out of date, but I think that would annoy people trying to improve PINT tremendously when all their PRs started failing. |
|
Quick list of observatories with clock corrections and the spans they cover: for a,o in pint.observatory.Observatory._registry.items():
m = o.last_clock_correction_mjd()
if np.isfinite(m):
print(f'{a:20s}\t{Time(m, format="mjd").iso} \t{(Time.now()-Time(o.last_clock_correction_mjd(), format="mjd")).datetime}')The second column is the date and time of the last correction, the third is how far in the past it is. Negative numbers, yes, mean values in the future. |
aarchiba
changed the title
|
I think reading TEMPO/TEMPO2 directories is maybe better put off to #1180 now that we have all the files from those distributions. If people have their own custom clock files they can put them in the PINT runtime directory. |
aarchiba
added
the
awaiting review
|
Suggestion in telecon: add documentation describing how PINT finds/handles clock corrections, including the new tools for checking when they end. Also explain what you need to do to use updated clock files (it involves editing PINT source). |
|
Suggestion: allow observatories to specify |
|
Now available: convenience functions. pint.observatory.check_for_new_clock_files_in_tempo12_repos()gives and pint.observatory.list_last_correction_mjds()gives |
|
Hi All: Sorry that I missed the discussion about this due to my travels. I wanted to bring up that I'm not in favor of copying all of the TEMPO/Tempo2 clock files into PINT. To me that is basically exacerbating the problem that we have in the pulsar community with clock files -- it now means that clock files will likely be out of date in two or three packages at a time rather than one or two. And it means that we need to update every time TEMPO/Tempo2 updates. This is something that Jing and others of us talked about in the early days of PINT and decided explicitly to avoid. IMO, if people want to use non-NANOGrav (or non-supported observatory) clock files, where they are primary in PINT, they should download TEMPO or Tempo2 and use those clock files as they like. I think we should be constantly moving towards single locations where the latest observatory clock files are kept, and we use a caching mechanism to handle them all. This copying of files seems like a step backwards to me, and I don't think we should hold the hands of our users who want to use "non-standard" clock files. |
|
I agree that we should be moving towards a centralized repository; as I said in #1180 I am working on a rough draft of how that might work (it's at https://github.com/aarchiba/pulsar-clock-corrections but let me emphasize that it is a proof of concept and I need to discuss what PINT support would look like - perhaps during the busy day?). If I understand correctly, your argument is that we should restrict PINT to having clock corrections only for a short list of observatories. Can you suggest such a list? Before this PR PINT had clock corrections for:
This PR adds:
It would seem that your preference is that PINT be a NANOGrav-specific tool, with users of other instruments required to install additional software to use it? As it currently stands (barring the sort of solution I mention above and we are discussing in #1180), PINT does not have any kind of search path. Thus with the current code, PINT developers must decide for each observatory between four options:
With this PR, observatories pointed at 2 or 3 emit a message and fall back to 4. (I don't get the impression that you have any problem with this.) This PR also moves all observatories from 2 and 3 to 1. As I understand it, this is the part you have a problem with. I will point out that PINT also has a tool to easily check and update the clock files to track those in the TEMPO/TEMPO2 repository, and that this is a temporary measure awaiting a more global solution (and probably also a fallback if getting files across the Net is inconvenient). Users who want custom clock files that aren't in any repository are in a difficult position with PINT, before or after this change; this does not attempt to solve their problem. I have some ideas on how to address this, but it's beyond the scope of this change. |
|
A document to discuss global clock files: https://docs.google.com/document/d/1St1hj0qqi8xsWpeMYUJGQ5zgITJEVoZ6bygicycROhU/edit?usp=sharing |
|
I truly don't think that PINT should have any clock files at all. Not even the NANOGrav ones (although given that PINT development has been NANOGrav-driven, I wouldn't be super opposed to keeping GBT, VLA, CHIME, and AO ones, but don't even think that is a good idea). So I'd be much more in favor of your (full) PR if it removed the ones that we had rather than adding new ones. You mention this as a short-term solution, but I really don't think we can count on that. And this is one of the main problems. Moving these clock files into PINT does increase the maintenance burden since we do not have access to the providers of those files, and so we need to depend on Tempo or Tempo2 commits before updating ours. I really do think it is just making the problem worse. Note that I have no problem explicitly always using the T2 runtime directory (if available) and falling back on the $TEMPO clock dir for clock files. Those packages are where the pulsar community gets most of their clock corrections now, so it isn't outrageous to demand that our users simply download those packages (they don't even need to install them!). Also note that one of the reasons why we started including GBT and AO clock files in PINT is because it is much harder to keep them up-to-date in TEMPO/Tempo2 as the PINT developers didn't all have write access to the TEMPO/Tempo2 repos. And we were using PINT for up-to-date NANOGrav development. So including them was a "short term solution" that is still (wrongly, IMO) being used. I'll take a look at your global clock files doc. |
|
TL;DR of thoughts on this: I love basically everything else in your PR and think they are all good improvements, except for adding additional clock files (and, once again, would prefer if your PR removed some or all of the ones we have!). |
|
I have a not-quite-PR-ready version of a draft of the global clock files machinery for PINT. But I think where we differ is on what to do until it's ready. Your suggestion is that we reduce the functionality of PINT in the meantime, or at least refuse to improve it. I think we should make it as usable as possible while we try to get the global clock corrections up and running. Speaking of which, where is the GBT static clock file location? I can automate pulling it in to the pulsar_clock_corrections repository (which will result in my not-quite-PR-ready version pulling it in immediately). |
|
"at least refuse to improve it" Hahaha. Interesting choice of words. I think this would possibly improve the usability of PINT for a small subset of folks in the short-term, but sets us back in multiple other ways (including maintenance). One point which hasn't been mentioned is that doing precise tests (especially with CI) is trickier if we don't have all of our own clock files, but would be doable with a bit of work. As for the GBT clock corrections, they are here: https://www.gb.nrao.edu/~fghigo/timer/time_gbt.dat |
|
It certainly improves the usability of PINT for me! I can now just use Parkes and Meerkat TOAs (the motivating example) and have them corrected. Issues around CI and requiring a network connection are more properly the business of the global-clock-corrections tool. The question for now is: what do we do until that is available? |
|
Right. But that same improvement would happen with your PR if you just had it use your Tempo/Tempo2 clock directories. Why is that not a better solution requiring less maintenance? And isn't that even an option in your PR? i.e. options 2 and 3 in your long explanation above? In fact, it already works now! I just loaded MeerKAT TOAs and the clock corrections were automatically grabbed from the T2 runtime directory. So I don't really think this is much of an improvement at all. |
|
Similarly, I've been automatically using Parkes clock corrections from the T2 runtime directory seamlessly for the last month or two while I've been working on |
|
Totalliy minor but I stopped the tiresome ERFA warning about a bad year in |
|
As I've mentioned, I'd much prefer that you just remove the Jodrell corrections (especially since they are effectively useless now) and replace them with the tempo2 runtime pointer. That's what we've done with the vast majority of other telescopes. As for the global clock repo, I don't understand why we'd need to wait on that. As an absolute worst case, we keep it up-to-date with TEMPO / Tempo2 until we get folks used to updating their telescopes in it themselves (or providing a link where those corrections can be pulled, ala GBT). And keeping some clock files up to date is already the situation we are in for PINT. I really don't buy the argument that PINT should be usable out of the box, because it isn't already, and never will be. We will always need to pull ephemerides and Earth rotation info and leap seconds (for instance). I don't see how clock corrections are any different. And having us include yet another copy of likely out of date ones is (IMO) a step backwards. Would love to hear what @paulray and @demorest think about this. Also, as for Sourceforge, I read the wiki page and it seems like most of those issues have been resolved or have gone away. I've tried to get David N and the other TEMPO devs (not me) to move the main repo to github in the past without success. So I doubt that it will change, and in the meantime that is the main repo. |
|
If you want the global clock corrections to happen soon, go discuss with me on #1247. There's a lot to do. In the meantime, let's keep PINT as functional as possible. If the meantime isn't very long, hooray! But let's not remove functionality. |
|
Fine. Then just forget the Jodrell additions, and change the default to Tempo2. That improves functionality and does't add a ton of very-soon-to-be-useless cruft: |
If I change the "default" to TEMPO2 users can no longer even use the Jodrell clock corrections in PINT. We don't have search paths. Who cares how many lines are in the git repo? It was created specifically to be fast for the Linux kernel source tree (legend has it Linus Torvalds wrote git while he was waiting for the existing source control tool to do some operation on the repo). This seems like a weird point to block merging of this PR. |
That's exactly the point. The Jodrell clock corrections in PINT right now should never have been put in there. I have no idea why they are even there (unless it was for a very limited test case for observatory handling). We should treat that file as a bug and fix the issue. (And yes, your solution does "fix" the issue, but with other problems, IMO)
That's the symptom. The problem is having duplicated (from TEMPO and Tempo2) and out-of-date clock files in PINT. (However, there is substantial impact for stuff like this going into and out of a repo. It definitely accumulates over time. Just look at the NANOGrav timing repos which are way bigger than they should be because of our changes of many large text files.) |
|
Please merge and I'll tag a new version. |
|
I resolved the conflict using the web-resolver. Feel free to tag-away, @paulray |
|
yikes. I was in the midst of resolving them locally. I'd better figure out what kind of situation the repo is in. |
|
It was literally just a one-line conflict with |
|
Thanks! |
This is meant to be a modest change, not yet implementing any global clock-file updating scheme as suggested in #1180 but addressing as far as possible the issue of finding any local clock files you do have. It also does something to address #1235 by pulling in all observatories that are in the TEMPO/TEMPO2 official repositories.
Notable changes:
get_TOAs("file.tim", limits="error")check_for_new_clock_files_in_tempo12_reposis a first attempt at comparing PINT's clock corrections to the versions on the WebGoals:
- [ ] PINT pulls newer/extra clock corrections from$TEMPO/or$TEMPO2/if availableCloses #1241 #1235