Correction to docstring for pbprime functions

[coclar]

While going through the binary orbit classes to implement a new model, I found that the docstrings for the pbprime() methods had an incorrect description. These should calculate the orbital period as a function of time, but the docstrings said that they compute the time-derivative of the orbital period (this is actually computed in pbdot_orbit()).

[dlakaplan]

Thanks for the fix. Do you think that this is used correctly otherwise?

[coclar]

In binary_orbits.py, yes - pbprime() is always correctly used as the orbital period.

However, after digging a bit deeper, there are some lines that I am unsure about in binary_generic.py:

The pb() and pbdot() methods are used by the binary models (ELL1model, BTmodel, etc.) obtain pbprime() and pbdot_orbit() from the relevant orbits_cls:
https://github.com/nanograv/PINT/blob/40dec33c013096096dd335c221393945033a3a66/src/pint/models/stand_alone_psr_binaries/binary_generic.py#L424C1-L425C41

https://github.com/nanograv/PINT/blob/40dec33c013096096dd335c221393945033a3a66/src/pint/models/stand_alone_psr_binaries/binary_generic.py#L447C1-L448C45

but then the PSR_BINARY class also has its own pbprime() method:
https://github.com/nanograv/PINT/blob/40dec33c013096096dd335c221393945033a3a66/src/pint/models/stand_alone_psr_binaries/binary_generic.py#L681C5-L682C51
I'm really not sure what this is supposed to be doing!

self.pb() retrieves self.orbits_cls.pbprime(), which should already account for any PBDOT, but then self.pbdot() * self.tt0 is subtracted from it, which would presumably "undo" the PBDOT effects to arrive back at the initial orbital period? But even this is incorrect if there are higher-order orbital frequency derivatives (i.e. pbdot_orbit() is not a constant value).

I'm struggling to follow the flow here though - it's possible that this method is always overwritten by the inheriting classes in binary_orbits.py that implement their own pbprime()s, and is therefore never used.

[dlakaplan]

Sorry for the delay in dealing with this. Do you think you could rebase it on the latest master and put in an entry in CHANGELOG-unreleased.md?

[coclar]

No worries, done!

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (7275528) 68.25% compared to head (5e701fa) 68.25%.
Report is 3 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1606   +/-   ##
=======================================
  Coverage   68.25%   68.25%           
=======================================
  Files          99       99           
  Lines       22791    22791           
  Branches     3920     3920           
=======================================
  Hits        15556    15556           
  Misses       6274     6274           
  Partials      961      961           

Files Changed	Coverage Δ
...t/models/stand_alone_psr_binaries/binary_orbits.py	88.40% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.