nanograv / enterprise

ENTERPRISE (Enhanced Numerical Toolbox Enabling a Robust PulsaR Inference SuitE) is a pulsar timing analysis code, aimed at noise analysis, gravitational-wave searches, and timing model analysis.
https://enterprise.readthedocs.io
MIT License
64 stars 65 forks source link

Adding Barycentric Pulsar Unit Vectors (pos_t) to PintPulsar #295

Closed Hazboun6 closed 2 years ago

Hazboun6 commented 2 years ago

This PR adds in real values for the Pulsar.pos_t vector. This enables the use of this vector for SSB modeling. Changes include:

self._pos_t = model.components[which_astrometry].ssb_to_psb_xyz_ICRS(model.get_barycentric_toas(toas)).value


- Expanding the PintPulsar tests to include all of the tests that exist for `TempoPulsar`
- Removing an exception that prohibited `PintPulsar`s from being used in BayesEphem
- Expanding the deterministic signal tests to include a test of BayesEphem with `PintPulsar`

(sorry about the weird history here, but the actual code changes are pretty minimal)
Hazboun6 commented 2 years ago

@AaronDJohnson Tests passed locally for me, and assuming they pass here, it would be worth trying this branch out on the 12.5 year analysis to see if you can reproduce the BayesEphem results with PintPulsars.

codecov[bot] commented 2 years ago

Codecov Report

Merging #295 (c09db26) into master (d7bc0f6) will increase coverage by 0.04%. The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #295      +/-   ##
==========================================
+ Coverage   86.95%   87.00%   +0.04%     
==========================================
  Files          13       13              
  Lines        2868     2870       +2     
==========================================
+ Hits         2494     2497       +3     
+ Misses        374      373       -1     
Impacted Files Coverage Δ
enterprise/signals/deterministic_signals.py 100.00% <ø> (ø)
enterprise/pulsar.py 92.36% <90.00%> (+0.39%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d7bc0f6...c09db26. Read the comment docs.

Hazboun6 commented 2 years ago

@AaronDJohnson is going to start a run with to test BayesEphem. I'd like to keep this PR open for fixes until we see the results of that analysis. I can try and add a couple of tests to bring the patch coverage up in the meantime, though it looks like that isn't necessary to merge.

vallis commented 2 years ago

One check would be to instantiate a PINT and a tempo2 PTA, and compare get_delay() for the same parameters.

Also, enabling the test_deterministic BE test for PINT should fix the coverage

AaronDJohnson commented 2 years ago

For every pulsar that I can check, the get_delay() values are an exact match (to all digits) between the PINT and TEMPO2 versions of the 12.5 year data when using BayesEphem for the same parameters. For some reason there are two pulsars that are not working in PINT for me right now on either my Mac or my Linux computers. I'll open an issue describing what is happening with those.

Hazboun6 commented 2 years ago

For every pulsar that I can check, the get_delay() values are an exact match (to all digits) between the PINT and TEMPO2 versions of the 12.5 year data when using BayesEphem for the same parameters.

That is fantastic!