Moved get_derived_params to timing_model

[dlakaplan]

Previously get_derived_params() was part of a Fitter object. This meant you had to load in a set of TOAs just to compute various quantities. As far as I could tell the only rationale for this was needing the RMS and number of TOAs to check for the ELL1 validity: everything else was only using the timing model parameters.

So I moved the function to timing_model, and it can accept optional inputs with the RMS and number of TOAs to do the ELL1 check. There is then a stub in the Fitter to maintain the same API.

I also simplified the function a bit and made better use of uncertainties. Some quantities like period and binary period are only determined once (rather than in multiple places) and almost all calculations use the uncertainties module.

It can also optionally output a dictionary of the parameters that it computed.

The get_derived_params() function is now tested on all par files in the test directory that are readable. If we add more types (more binary models) then there will be additional tests.

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (fc61ed6) 68.41% compared to head (d6634b7) 68.50%.

Files	Patch %	Lines
src/pint/models/timing_model.py	97.16%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1692      +/-   ##
==========================================
+ Coverage   68.41%   68.50%   +0.09%     
==========================================
  Files         104      104              
  Lines       24339    24345       +6     
  Branches     4346     4342       -4     
==========================================
+ Hits        16651    16678      +27     
+ Misses       6600     6588      -12     
+ Partials     1088     1079       -9     

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

[dlakaplan]

Example:

In [1]: from pint.models import get_model

In [2]: m=get_model("tests/datafile/J0023+0923_NANOGrav_11yv0.gls.par")

In [3]: print(m.get_derived_params())
Derived Parameters:
Period = 0.003050203104480002±0.000000000000000006 s
Pdot = (1.142342±0.000032)×10⁻²⁰
Characteristic age = 4.231e+09 yr (braking index = 3)
Surface magnetic field = 1.89e+08 G
Magnetic field at light cylinder = 6.032e+04 G
Spindown Edot = 1.589e+34 erg / s (I=1e+45 cm2 g)

Parallax distance = 1075±180 pc

Binary model BinaryELL1
Orbital Period  (PB) = 0.138799143319±0.000000000034 (d)
Orbital Pdot (PBDOT) = (2.68±0.20)×10⁻¹² (s/s)
Conversion from ELL1 parameters:
ECC = (8.2±3.1)×10⁻⁶
OM  = 119±22 deg
T0  = 56567.072(8)

Mass function = 2.357201(13)×10⁻⁶ Msun
Min / Median Companion mass (assuming Mpsr = 1.4 Msun) = 0.0168 / 0.0194 Msun


In [4]: _,o=m.get_derived_params(returndict=True)

In [5]: o
Out[5]: 
{'P (s)': 0.003050203104480002+/-6.275371941051834e-18,
 'Pdot (s/s)': 1.1423421273063399e-20+/-3.176274686405792e-25,
 'age': <Quantity 4.23056725e+09 yr>,
 'B': <Quantity 1.88891517e+08 G>,
 'Blc': <Quantity 60321.78682038 G>,
 'Edot': <Quantity 60321.78682038 G>,
 'Dist (pc)': 1075.1532093323297+/-179.98210374480567,
 'Binary': 'BinaryELL1',
 'PB (d)': 0.1387991433191316+/-3.43775032248505e-11,
 'PBDOT (s/s)': 2.684035746887999e-12+/-1.9922464497128316e-13,
 'ECC': 8.206501102175031e-06+/-3.0540728551393467e-06,
 'OM (deg)': 119.31907495262094+/-21.649059517672388,
 'T0': 56567.07209747229+/-0.008346863652321972,
 'Mass Function (Msun)': 2.357201023902326e-06+/-1.3192752414993872e-11,
 'Mc,med (Msun)': 0.019409155678639364,
 'Mc,min (Msun)': 0.016788123112625043}

[dlakaplan]

It is slightly clumsy that the dictionary returns a mix of Quantities and uncertainties values, with the units for the later only present in the dictionary keys. But this seemed simplest.

[scottransom]

Yeah, maybe a bit clumsy. But looks super useful, though! Thanks for doing this!

[dlakaplan]

Would it make sense to change the dictionary output to more like:

'param': {'value': ..., 'uncertainty': ..., 'unit': ...}

this would allow for easy serialization that could even be extended to the whole timing model. But it may also be overly complex for this case which is just mean for humans to read.

[abhisrkckl]

This looks good. Shall I merge this?

[dlakaplan]

Probably. Depends if there is any thought about if/how to change the dict output. But that could also be a separate PR.

[abhisrkckl]

I think that can be done separately.

[dlakaplan]

OK, then I think you can go ahead and merge if it looks OK.