Test cleanup and simplify special_locations

[aarchiba]

This PR is meant to remove most of the places where tests are skipped because $TEMPO2 is not (or is) set. For the most part, these were because they depended on clock files in TEMPO2 but not in PINT; this is no longer a concern. There are a few other situations where this sort of thing comes up, but the PINT test suite does not run tempo2 even if it is available - where direct comparisons are made, they are to saved tempo2 output. (It is not always evident how this output can be regenerated.)

This PR also clears up some anomalies in how missing clock files were handled, including some changes to SpecialLocations. This fixes some problems/inconsistencies revealed in the work on #1287. More importantly it returns all SpecialLocations to how they behaved before the global clock corrections were merged (that had disabled GPS and BIPM corrections). Just to be clear: the global clock corrections broke (inadvertently changed) how we handled SpecialLocations, and this PR fixes that.

This PR also cleans up a number (all?) of our tests that are marked xfail but actually pass. Mostly this is the results of bug fixes and other improvements resolving what used to be problems, so the tests should now alert us to regressions.

Closes #1304

[codecov]

Codecov Report

Merging #1305 (1a456ba) into master (36d1fe9) will increase coverage by 0.22%.
The diff coverage is 86.08%.

@@            Coverage Diff             @@
##           master    #1305      +/-   ##
==========================================
+ Coverage   62.00%   62.22%   +0.22%     
==========================================
  Files          89       89              
  Lines       20273    20215      -58     
  Branches     3658     3650       -8     
==========================================
+ Hits        12570    12579       +9     
+ Misses       6918     6849      -69     
- Partials      785      787       +2     

Impacted Files	Coverage Δ
src/pint/observatory/clock_file.py	77.39% <66.66%> (-1.61%)	⬇️
src/pint/observatory/__init__.py	59.58% <84.26%> (+8.59%)	⬆️
src/pint/observatory/topo_obs.py	80.14% <93.75%> (+0.88%)	⬆️
src/pint/observatory/satellite_obs.py	66.31% <100.00%> (+0.18%)	⬆️
src/pint/observatory/special_locations.py	57.69% <100.00%> (-4.28%)	⬇️
src/pint/residuals.py	66.66% <0.00%> (-5.20%)	⬇️
src/pint/toa.py	83.23% <0.00%> (+0.30%)	⬆️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

[paulray]

Nice to see all this cleanup!

[aarchiba]

The newly added FIXMEs are waiting on #1287 and the plumbing that comes with it (so you can temporarily create an observatory without polluting the registry of observatories)

[aarchiba]

Outcome of PINT meeting discussion:

• Exactly which timescales satellites use is not always clear, but even if they are ostensibly TT we might want to apply the GPS post-facto correction and the TT->TT(BIPM2021) correction for some
• Not clear whether this is exactly what include_bipm does - there might be an offset
• Geocenter TOAs could be any timescale depending on who made them and how
• Our handling of the include_gps and inlclude_bipm flags through global changes to observatories is problematic but a big job to change
• SpecialLocations do need the include_gps and include_bipm flags.

[aarchiba]

There is an additional layer of confusion. PINT Observatory objects have a timescale attribute that supposedly "Returns the timescale that TOAs from this observatory will be in, once any clock corrections have been applied." This is decidedly weird when the clock corrections include not just observatory clock to UTC but also GPS forecast to UTC and TT(TAI) to TT(BIPM2021). So the clock corrections are applied in a slightly confusing order.

The problem is that TopoObs by default report "utc", Barycenter reports "tdb" , Goecenter reports "utc", and T2SpacecraftObs report "utc".

This timescale is passed to Astropy for further conversion, including in particular leap second removal for UTC. I think satellite observations should probably be assumed to be in TDB? But things are working now so maybe that is ignored?

@paulray @kerrm Any comments?

[paulray]

For NICER, non-barycentered photon event files have times natively in TT. Here are the relevant header keywords in the FITS file:

TIMESYS = 'TT      '           / Reference time system
MJDREFI =                56658 /  [d] MJD reference day (2014-01-01T00:00:00)
MJDREFF = 0.000777592592592593 / [d] MJD reference (fraction of day)
TIMEREF = 'LOCAL   '           / Reference time location
TASSIGN = 'SATELLITE'          / Time assigned by clock
TIMEUNIT= 's       '           / unit for time keywords
TIMEZERO=                  -1. / Time Zero
LEAPINIT=                    2 / [s] Leap seconds between MJDREF and TSTART

Now, event_toas.py handles reading those files and it sets the scale = 'tt' for this type of file.
Here is what happens when you load a NICER observatory and event file:

In [9]: nicer = pint.observatory.satellite_obs.get_satellite_observatory("NICER","../../auxil/ni5202840101.orb")
INFO     (pint.observatory.satellite_obs): Opened FPorbit FITS file ../../auxil/ni5202840101.orb
DEBUG    (pint.observatory.satellite_obs): FPorbit TIMESYS TT
DEBUG    (pint.observatory.satellite_obs): FPorbit TIMEREF LOCAL
DEBUG    (pint.fits_utils               ): TIMEZERO = 0.0
DEBUG    (pint.fits_utils               ): MJDREF = 56658.00077759259
DEBUG    (pint.observatory.satellite_obs): FPorbit spacing is 9.99999982304871 s
INFO     (pint.observatory.satellite_obs): Building FPorbit table covering MJDs 59749.03290374419 d to 59749.95547318864 d
DEBUG    (pint.observatory.satellite_obs): Sorting FPorbit table

In [10]: ts = pint.event_toas.load_NICER_TOAs("ni5202840101_0mpu7_cl.evt")
DEBUG    (pint.event_toas               ): TIMESYS TT
DEBUG    (pint.event_toas               ): TIMEREF LOCAL
INFO     (pint.event_toas               ): Building spacecraft local TOAs
DEBUG    (pint.fits_utils               ): TIMEZERO = -1.0
DEBUG    (pint.fits_utils               ): MJDREF = 56658.00077759259

In [18]: ts[1].mjd
Out[18]: <Time object: scale='tt' format='mjd' value=59749.61486327273>

So, TOAs from spacecraft just come in as TT times and should never go through a UTC to TT conversion.
For some spacecraft (like NICER), those TT times are referenced to an onboard GPS receiver, so it is appropriate to apply gps2utc.clk if the user wants it. It is also fine to apply TT(TAI) to TT(BIPM), as long as that gets done correctly (but this I have not checked).

[paulray]

Oh, I should add that we only use satellite_obs. This is entirely different than T2SpaceCraftObs. I'm not aware of anyone who has used that in PINT. It likely does assume TOAs are in UTC time.

[paulray]

One other note is that our satellite_obs TOAs can't be represented in a .tim file, since (as far as I know) MJDs in .tim files are always UTC (except for the @ special case)

[aarchiba]

Aha, thanks! I haven't touched spacecraft_obs, so I don't know what all happens there.

PINT allows for the possibility that special locations specified in .tim files could be in other timescales; or with a bit of coding, topocentric observatories that gave us non-UTC time scales could be accommodated. But I think that the .timescale attribute is not actually being a problem, if no one is using T2SpacecraftObs.

The corrections applied by include_gps and include_bipm are both appropriate for applying to already-TT times if they are computed from GPS without those corrections already applied; so using the Observatory options to do that should be fine.

I think that prior to my interference, satellite_obs did in fact apply those corrections, and they definitely claimed a timescale of TT. I don't think I have broken it with the current version.

[aarchiba]

I think I can say more: If you give your satellite observatory a reasonable name, you should be able to write out your TOAs to a tim file, and then as long as the satellite observatory was available with the same name, anyone could read in your .tim files and they would be interpreted correctly. At least in tempo2 format, there's some weirdness in Princeton format I've been hesitant to touch, but tempo2 format at least tries to construct the Time object with the observatory's .timescale value.

There might be some weirdness with the GPS/BIPM clock corrections as TOAs objects are supposed to apply those when reading in and record their values so they can be removed when writing back out. If you make your TOAs object in a slightly unusual way they might not ever get applied.

[aarchiba]

I believe that this PR does not change how any special locations behave, compared to the pre-global-clock-corrections code.

[kerrm]

I concur that I think things are OK for satellites. I checked out the branch and ran some tests with local data, everything was exactly the same. After chatting with Paul, we have an idea to improve test_fermiphase.py (and some others could also benefit!) that would have failed if anything deleterious had happened with this PR.

[paulray]

I removed the awaiting review label. This can be merged once the conflicts are resolved and the tests pass.