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

TCB to TDB conversion #1531

Merged
merged 56 commits into from
Apr 13, 2023
Merged

TCB to TDB conversion #1531

merged 56 commits into from
Apr 13, 2023

Conversation

abhisrkckl
Copy link
Contributor

@abhisrkckl abhisrkckl commented Feb 13, 2023

This PR provides a TCB to TDB conversion tool (tcb2tdb) which works like the transform plugin of tempo2.

See issues #1405 and #37 for more details. There has been a lot of discussion in the PINT community about TCB support, and the consensus is to not provide any TCB support internally. i.e., the internal representation of TOAs, frequencies, delays etc will only use the TDB timescale. However, a TCB to TDB conversion feature is useful because there are a lot of TCB par files in the wild and the only tool for this conversion at present is tempo2 -gr transform.

Note that this conversion is only approximate and the par files should not be used without re-fitting. Nevertheless, this conversion should retain the phase connection so that the re-fitting is not too painful. The approximate nature of the conversion is also applicable to the transform plugin.

The following parameters are converted to TDB:
1. Spin frequency, its derivatives and spin epoch
2. Proper motion and the position epoch
3. DM, DM derivatives, DM epoch
4. Keplerian binary parameters and FD1

The following parameters are NOT converted although they in fact affected by the TCB to TDB conversion:
1. Parallax
2. TZRMJD and TZRFRQ
2. DMX parameters
3. Solar wind parameters
5. Binary post-Keplerian parameters including Shapiro delay parameters (except FD1)
6. Jumps and DM Jumps
7. FD parameters
8. EQUADs
9. Red noise parameters including FITWAVES, powerlaw red noise powerlaw DM noise

It is pretty easy to implement TCB conversion on read if people think that is appropriate.

Edit 23-03-2023:
The get_model, get_model_and_toas, and ModelBuilder.__call__ functions now have an option (allow_tcb) to convert TCB par files to TDB on read. If allow_tcb=False, these functions will throw an error upon encountering TCB. If allow_tcb=True, they will convert the model upon read and emit a warning about the approximate nature of the conversion. If allow_tcb="raw", the functions will simply create a malformed TimingModel object with TCB units that cannot be used for anything other than converting to TDB. The tcb2tdb script simply calls the get_model function and writes the converted model into a par file.

@abhisrkckl abhisrkckl changed the title TCB to TDB conversion WIP: TCB to TDB conversion Feb 13, 2023
@codecov
Copy link

codecov bot commented Feb 13, 2023

Codecov Report

Patch coverage: 83.24% and project coverage change: +0.11 🎉

Comparison is base (ba659d9) 67.61% compared to head (0976fa4) 67.72%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1531      +/-   ##
==========================================
+ Coverage   67.61%   67.72%   +0.11%     
==========================================
  Files          95       97       +2     
  Lines       22791    22849      +58     
  Branches     3948     3952       +4     
==========================================
+ Hits        15409    15474      +65     
+ Misses       6411     6401      -10     
- Partials      971      974       +3     
Impacted Files Coverage Δ
src/pint/models/solar_system_shapiro.py 94.44% <0.00%> (ø)
src/pint/models/timing_model.py 83.24% <72.97%> (+0.25%) ⬆️
src/pint/models/tcb_conversion.py 83.92% <83.92%> (ø)
src/pint/models/model_builder.py 94.22% <95.45%> (-0.06%) ⬇️
src/pint/scripts/tcb2tdb.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@abhisrkckl abhisrkckl changed the title WIP: TCB to TDB conversion TCB to TDB conversion Feb 14, 2023
@paulray
Copy link
Member

paulray commented Mar 1, 2023

Just FYI, I just pulled this and tried it on a TCB par file and it seemed to work. However, the last warning was a little odd because it is not true. The input par file was TCB:

% tcb2tdb J0659+1414_NRT_20190322.par J0659+1414_NRT_20190322_TDB.par          

WARNING  (pint.logging                  ): /Users/paulr/src/PINT/src/pint/models/timing_model.py:367 UserWarning: PINT does not support 'DILATEFREQ Y'
WARNING  (pint.logging                  ): /Users/paulr/src/PINT/src/pint/models/timing_model.py:370 UserWarning: PINT only supports 'TIMEEPH FB90'
WARNING  (pint.models.timing_model      ): PINT does not support 'UNITS TCB' internally. Reading this par file neverthelessbecause the `allow_tcb` option was given. This `TimingModel` object should not beused for anything except converting to TDB.
WARNING  (pint.logging                  ): /Users/paulr/src/PINT/src/pint/models/timing_model.py:399 UserWarning: START cannot be unfrozen... Setting START.frozen to True
WARNING  (pint.logging                  ): /Users/paulr/src/PINT/src/pint/models/timing_model.py:402 UserWarning: FINISH cannot be unfrozen... Setting FINISH.frozen to True
WARNING  (pint.logging                  ): /Users/paulr/src/PINT/src/pint/models/model_builder.py:130 UserWarning: Unrecognized parfile line 'EPHVER 5'
WARNING  (pint.models.tcb_conversion    ): The TCB to TDB conversion is only approximate. The resulting timing model should be re-fit to get reliable results.
WARNING  (pint.models.tcb_conversion    ): The input par file is already in the target units. Doing nothing.
INFO     (pint.scripts.tcb2tdb          ): Output written to J0659+1414_NRT_20190322_TDB.par.

@paulray
Copy link
Member

paulray commented Mar 1, 2023

I notice that the roundtrip test is failing.

@dlakaplan
Copy link
Contributor

@abhisrkckl : can you share the log messages like the ones @paulray showed above? It would be good to see what they look like if they have changed at all.

@abhisrkckl
Copy link
Contributor Author

abhisrkckl commented Apr 6, 2023

@dlakaplan I have fixed this. The log messages now look like this:

$ tcb2tdb test.par test_tdb.par
WARNING (pint.models.timing_model ): PINT does not support 'UNITS TCB' internally. Reading this par file neverthelessbecause the allow_tcb option was given. This TimingModel object should not beused for anything except converting to TDB.
WARNING (pint.models.tcb_conversion ): The TCB to TDB conversion is only approximate. The resulting timing model should be re-fit to get reliable results.
INFO (pint.scripts.tcb2tdb ): Output written to test_tdb.par.

@abhisrkckl
Copy link
Contributor Author

Improved log messages from tcb2tdb:

$ tcb2tdb test.par test_tdb.par
WARNING (pint.models.timing_model ): PINT does not support 'UNITS TCB' internally. Reading this par file nevertheless because the allow_tcb option was given. This TimingModel object should not be used for anything except converting to TDB.
WARNING (pint.models.tcb_conversion ): Converting this timing model from TCB to TDB. Please note that the TCB to TDB conversion is only approximate and the resulting timing model should be re-fit to get reliable results.
INFO (pint.scripts.tcb2tdb ): Output written to test_tdb.par.

@abhisrkckl
Copy link
Contributor Author

I have also updated the Explanation page to reflect these changes.

@paulray
Copy link
Member

paulray commented Apr 13, 2023

I'd like to merge this now. Is that fine?

@abhisrkckl
Copy link
Contributor Author

@paulray Yes.

@paulray paulray merged commit 489ba29 into nanograv:master Apr 13, 2023
@abhisrkckl abhisrkckl deleted the tcb2tdb branch May 8, 2023 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review This PR needs someone to review it so it can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PINT should have a TCB to TDB conversion script
5 participants