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

TOA reading as a model method? #748

Closed
scottransom opened this issue Jul 15, 2020 · 3 comments
Closed

TOA reading as a model method? #748

scottransom opened this issue Jul 15, 2020 · 3 comments
Labels
enhancement student Good student project

Comments

@scottransom
Copy link
Member

scottransom commented Jul 15, 2020

So in our example fit-NGC6440E.py, we currently read the model first and then read the TOAs. But we don't use the information that is (potentially) in the model to do a better job reading to TOAs. For instance, we ignore the ephemeris used and also whether the planetary shapiro delay is going to be used or not.

Should we make using the model information an option for get_TOAs()? Or alternatively make a model method that uses the information to call get_TOAs() better with all of the appropriate keyword arguments set properly? Or maybe even both?

For example, instead of:

# Define the timing model
m = mb.get_model(parfile)
# Read in the TOAs
t = pint.toa.get_TOAs(timfile)

which can improperly load in TOAs without proper information from the model, we would do either:

# Define the timing model
m = mb.get_model(parfile)
# Read in the TOAs
t = pint.toa.get_TOAs(timfile, model=m)

or

# Define the timing model
m = mb.get_model(parfile)
# Read in the TOAs
t = m.get_TOAs_with_model(timfile)

?

@scottransom scottransom added enhancement student Good student project labels Jul 15, 2020
@paulray
Copy link
Member

paulray commented Jul 16, 2020

That is a very good suggestion! Currently, something like pintempo has to do a bunch of checks to deal with various situations like setting ephemeris, adding TIM file JUMPs to the model, and computing pulse numbers is TRACK mode is on:

    use_planets = False
    if m.PLANET_SHAPIRO.value:
        use_planets = True
    model_ephem = "DE421"
    if m.EPHEM is not None:
        model_ephem = m.EPHEM.value
    t = pint.toa.get_TOAs(args.timfile, planets=use_planets, ephem=model_ephem)

    # turns pre-existing jump flags in t.table['flags'] into parameters in parfile
    m.jump_flags_to_params(t)

    if m.TRACK.value == "-2":
        if "pn" in t.table.colnames:
            log.info("Already have pulse numbers from TOA flags.")
        else:
            log.info("Adding pulse numbers")
            t.compute_pulse_numbers(m)

All this kind of stuff could be handled in one place, as you suggest.

@luojing1211
Copy link
Member

Are we going to make a component for these control parameters? If that is the case, we can add validate() and other fun stuff.

@abhisrkckl
Copy link
Contributor

This functionality is implemented in the get_model_and_toas() function.

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

No branches or pull requests

4 participants