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

the correct way to get_PSR_freq #713

Open
luojing1211 opened this issue May 22, 2020 · 11 comments
Open

the correct way to get_PSR_freq #713

luojing1211 opened this issue May 22, 2020 · 11 comments
Assignees

Comments

@luojing1211
Copy link
Member

Currently get_PSR_freq(modelF0=True) only returns F0 in the model, however, should we get the F0 at TOA? F0(TOA) = F0 + dt * F1 +....
Secondly, this check seems duplicated with the Spindown component's validation.

PINT/src/pint/residuals.py

Lines 135 to 156 in 6fbbd92

if modelF0:
"""Return pulsar rotational frequency in Hz. model.F0 must be defined."""
if self.model.F0.units != "Hz":
ValueError("F0 units must be Hz")
# All residuals require the model pulsar frequency to be defined
F0names = ["F0", "nu"] # recognized parameter names, needs to be changed
nF0 = 0
for n in F0names:
if n in self.model.params:
F0 = getattr(self.model, n).value
nF0 += 1
if nF0 == 0:
raise ValueError(
"no PSR frequency parameter found; "
+ "valid names are %s" % F0names
)
if nF0 > 1:
raise ValueError(
"more than one PSR frequency parameter found; "
+ "should be only one from %s" % F0names
)
return F0 * u.Hz

@paulray
Copy link
Member

paulray commented May 22, 2020

When we need precise frequency at a TOA (e.g. in zima.py), I use this:

F_local = m.d_phase_d_toa(ts)

I seem to recall testing that in residuals but it makes it really slow. It is an outstanding problem that F0 is used for frequency in some places like this. It is often a sufficient approximation, but not always. I timed a pulsar that changed in period by a factor of two in a couple years.

@paulray
Copy link
Member

paulray commented May 22, 2020

See this discussion: #415

@luojing1211
Copy link
Member Author

Yeah, I remember that. Should we at least provide a Taylor expansion version of F0 at TOA? Plus, the check should use the spindown validation.

@paulray
Copy link
Member

paulray commented May 22, 2020

Sure, those changes would be fine with me.

@scottransom
Copy link
Member

Could he have it as an option to use the full F_local = m.d_phase_d_toa(ts) but use the quick normal version by default?

@luojing1211
Copy link
Member Author

I think this function should have three options:

  1. Return a quick F0 in the model
  2. F0(TOA) using Taylor expansion with F1, F2...
  3. The apparent F0, d_phase_d_TOA().

This function should be in the spin-down component.

@paulray
Copy link
Member

paulray commented May 22, 2020

That sounds fine. For option 3, it will need to be passed a TOAs object as an argument.
I think the functionality of getting the pulsar freq is used elsewhere in PINT, so we should make them all use this one function, with its three options depending on their needs for speed and/or accuracy.

@luojing1211
Copy link
Member Author

luojing1211 commented May 22, 2020 via email

@paulray
Copy link
Member

paulray commented Dec 9, 2020

Copying this from a different discussion with @aarchiba :

Ah, so there are four possible ways to handle phase -> time conversion:

  • Use the full delay calculation (at what time? before or after delay corrections?)
  • Use the Taylor series but don't worry about delay/timescales
  • Use F0 from the model
  • Use some fiducial F0 that is not a fit parameter

We should make a PR to address this, after #861 is merged. For the full delay calculation, isn't the right time just the topocentric time of the TOA? You want the observed apparent frequency of the pulsar to convert a phase offset to a time offset.

@abhisrkckl
Copy link
Contributor

@abhisrkckl
Copy link
Contributor

New improvements to this are in #1334

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants