-
Notifications
You must be signed in to change notification settings - Fork 67
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
base: dev
Are you sure you want to change the base?
Conversation
Shout out |
Could someone check if this is okay to merge? Maybe @AaronDJohnson or @vallis? |
There was a problem hiding this 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
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? |
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. |
This should be good to go now |
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 originalPulsar
object to identify timing parameter name (but you need this to make thePTA
object anyways).