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

add support for time dependent (and filter dependent) systematic error #383

Merged

Conversation

sahiljhawar
Copy link
Member

@sahiljhawar sahiljhawar commented Sep 21, 2024

Copy of #253

This PR adds support for sampled systematic error and time (and filter) dependent (interpolated) systematic error. For optimum performance it is recommended that the filters to be used for systematic biases should be less than or equal to the no of filters being used for the inference. If the no of filters for systematic biases is more than the no of --filters then the sampler will unnecessarily sample the priors which are of no use. For eg. if the filters in --filters is sdssu,ps1__g,ps1__r,ps1__i,ps1__z,ps1__y,2massj,2massh,2massks and you want to independently sample ps1__r and ps1__i filters then the yaml should be

config:
  withTime:
    value: true
    filters: ["ps1__r", "ps1__i", null]
    ...
    ...

null will be a single array which will be used for all other filters excluding ps1__r and ps1__i.

The option is still there to add the systematic error in the prior file, however changed the name of the prior to be sys_err

@mcoughlin
Copy link
Member

@sahiljhawar maybe you forgot a commit?

@sahiljhawar
Copy link
Member Author

@mcoughlin which commit?

@mcoughlin
Copy link
Member

@sahiljhawar i just notice the tests were failing due to a missing argument :)

@sahiljhawar
Copy link
Member Author

yeah, I just checked the tests were failing due to outdir I guess. That's why the new commit with autodelete outdir after each test.

Copy link
Member

@mcoughlin mcoughlin left a comment

Choose a reason for hiding this comment

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

This looks great. Can you add a page to the docs on this with maybe a few sample plots @sahiljhawar ?

@sahiljhawar
Copy link
Member Author

Yeah sure, I can do that.

@mcoughlin
Copy link
Member

@sahiljhawar any update herE?

@sahiljhawar
Copy link
Member Author

@mcoughlin Waiting for @tsunhopang review

@tsunhopang
Copy link
Collaborator

tsunhopang commented Oct 1, 2024 via email

@tsunhopang
Copy link
Collaborator

lgtm

@tsunhopang tsunhopang merged commit e036771 into nuclear-multimessenger-astronomy:main Oct 6, 2024
9 checks passed
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.

3 participants