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

Get TM processes from ConditionalGP #387

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

Conversation

blarsen10
Copy link

This PR allows you to input a list of timing model parameters to ConditionalGP and return the realizations of those individual timing model processes, alongside all other processes which use a basis. This is useful because we can now isolate realizations from individual timing processes (such as spindown or dispersion) from the rest of the timing model. The only catch is users will need to also input the original Pulsar object to identify timing parameter name (but you need this to make the PTA object anyways).

@blarsen10
Copy link
Author

Shout out la_forge.gp where this functionality is taken from

@blarsen10
Copy link
Author

Could someone check if this is okay to merge? Maybe @AaronDJohnson or @vallis?

Copy link
Member

@vhaasteren vhaasteren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good as far as I can see

@vhaasteren
Copy link
Member

Hi @blarsen10, sorry that this took time to review. Sometimes dev work can be on the backburner.

It looks fine, and I'm happy to merge. Do I see right that you have also tested this when sampling the coefficients explicitly?

@blarsen10
Copy link
Author

Thanks for looking at this @vhaasteren ! Glad you asked about the coefficients, I didn't test that part since I was interested in looking at the timing model realizations in the time domain. I realize now that I didn't update it correctly, it's currently adding a new key for every new TM parameter, but duplicating the set of all coefficients from the linearized timing model, instead of only outputting only the coefficient for that parameter. I'll update this so it works as expected and let you know when its ready to merge.

I'll add that I don't think the functionality to sample individual TM keys is strictly necessary for coefficients, since that information can already by isolated out when sampling the coefficients from the whole model (unlike the time-domain processes), but adding coefficients for the individual TM keys to the dictionary at least has the benefit that it will do the key-matching for you already.

@blarsen10
Copy link
Author

This should be good to go now

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

Successfully merging this pull request may close these issues.

2 participants