-
Notifications
You must be signed in to change notification settings - Fork 67
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
Adding PintPulsar SSB Vectors #244
Conversation
…jsh-pint-ssb Merging changes
Hmmmm... That seems further off than I would expect. And am I reading things correctly in that the errors only seem to be in Y and Z and not X? This will likely take some investigation. |
@scottransom yeah, that's right regarding the coordinate differences. Below is just the x direction difference, which is more like ~50 nanoseconds. |
Indeed, it would be good to understand this, although it won't affect BayesEphem, since the orbits of the planets are multiplied by M_planet/M_sun to yield mass-perturbation partials, and a few milliseconds out of thousands seconds will be a small correction. Where does PINT get planetary ephemerides? Are they coordinate-transformed before being reported? In other news, Jeff, could you merge master into the PR so you get the effects of the new continuous integration? |
@vallis So the high-level PINT routine that is called to get the planet positions is compute_posvels() in toa.py. That calls objPosVel_wrt_SSB() which is in solar_system_ephemerides.py. However, the code that actually gets the positions and velocities from the JPL ephemerides is from the astropy coordinates get_body_barycentric() method. So we probably need to dig in there (and perhaps compare with pyephem or some other codes). |
@vallis I merged the recent CI changes. So you think it's okay to merge this PR and start testing |
Codecov Report
@@ Coverage Diff @@
## master #244 +/- ##
==========================================
+ Coverage 85.56% 85.69% +0.12%
==========================================
Files 12 12
Lines 2723 2740 +17
==========================================
+ Hits 2330 2348 +18
+ Misses 393 392 -1
Continue to review full report at Codecov.
|
@paulray, @luojing1211 These tests are failing on py3.8 for some weird error with |
Yeah, we get things like that sometimes. There is a good chance that re-running that test will fix it. There is an East Coast-wide internet issue today so that could be related. |
The code appears to be correct, basically inverting PINT's subtraction to get back the Astropy output: versus https://github.com/nanograv/PINT/blob/daea8fc7a5bc8b8de974907abb4d87d9bd47c613/src/pint/toa.py#L2032 We can (should) have a round-trip test with PINT to confirm that our generated vectors do agree with Astropy, but if you are depending on PINT already you could add a check of PINT and TEMPO2 SSB-planet vectors against Astropy to the test suite here. |
Thanks @paulray rerunning did the trick! |
@aarchiba thank you for the code cross check. That is very helpful. |
This PR tries to fix Issue #238 and pulls out the needed vectors from the Pint timing objects in order to do solar system ephemeris modeling. The unit tests pass, but the vectors seem to be off on the order of milliseconds. Before merging I'd like to hear from the Pint team about if this sort of difference is expected. Below are two images that show the vectors and the difference (
libstempo
-Pint
) for Earth and Jupiter. These are for PSR J1911+1347 with the NANOGrav 12.5 year data set.