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

look for injections not only on the top-level in hdf file #4876

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WuShichao
Copy link
Contributor

Standard information about the request

This is a: bug fix

This change affects: inference, workflow

This change changes: plotting

This change will: fix the plotting injected values from mutliband demargin hdf file's injections

Motivation

the default operation is to look for injections on the top-level in hdf file, this fix lets it to look it under the primary model

Contents

lets it to look injections under the primary model

Links to any issues or associated PRs

Testing performed

Additional notes

  • The author of this pull request confirms they will adhere to the code of conduct

@WuShichao WuShichao changed the title Update __init__.py look for injections not only on the top-level in hdf file Sep 11, 2024
@ahnitz ahnitz requested review from cdcapano and removed request for ahnitz September 12, 2024 15:11
@ahnitz
Copy link
Member

ahnitz commented Sep 12, 2024

This has more to do with how we want to handle the heirarchical analysis. @cdcapano Should weigh in

@WuShichao
Copy link
Contributor Author

@ahnitz @cdcapano This PR fixes my current issue but can be more general for other hierarchical-based models.

Copy link
Contributor

@cdcapano cdcapano left a comment

Choose a reason for hiding this comment

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

Thanks for doing this; it's a good thing to fix. However, the patch as-is will only fix the case for the JointPrimaryMarginalized model, and will only load the injection from the primary model at that. It would be good to fix this for more generic hierarchical models. Currently, plotting injection parameters is broken for all hierarchical runs, since the injections are specific to each sub-model.

I think the way to go about this is to check if there's a submodels argument in the [model] section of the config file, and if so, get their names. Then cycle over all of the submodels and load the injection from each one, but remap the parameter names of the injections to <submodel_label>__<param>. You'd want to do that anyway since the template parameters will have the submodel name pre-pended to them. I think that should work for all models that inherit from the HierarchicalModel, including the JointPrimaryMarginalized, but you and @ahnitz should verify since you wrote the JointPrimary.

@WuShichao
Copy link
Contributor Author

Thanks for doing this; it's a good thing to fix. However, the patch as-is will only fix the case for the JointPrimaryMarginalized model, and will only load the injection from the primary model at that. It would be good to fix this for more generic hierarchical models. Currently, plotting injection parameters is broken for all hierarchical runs, since the injections are specific to each sub-model.

I think the way to go about this is to check if there's a submodels argument in the [model] section of the config file, and if so, get their names. Then cycle over all of the submodels and load the injection from each one, but remap the parameter names of the injections to <submodel_label>__<param>. You'd want to do that anyway since the template parameters will have the submodel name pre-pended to them. I think that should work for all models that inherit from the HierarchicalModel, including the JointPrimaryMarginalized, but you and @ahnitz should verify since you wrote the JointPrimary.

Thanks for the comments, my JointPrimaryMarginalized is different, in the config file it doesn't have submodels like the original HierarchicalModel:

[model]
name = joint_primary_marginalized
primary_model = 3g
other_models = lisa
static_margin_params_in_other_models = True

So fp.read_config_file() will not see submodels, but after initializing the model by from_config it will have self.submodels, while it will not be initialized during plotting.

@cdcapano
Copy link
Contributor

Ok, then add a static method to the HierarchicalModel, something like get_model_labels(cp), that loads the model names given a config file. In the HierarchicalModel that will look for the submodels argument. Override that method in the JointPrimaryMarginalizedModel to load the primary_model and other_models instead. In either case, the get_model_labels should return the model labels. Then in injections_from_cli you can just call get_model_labels without needing to worry about looking for specific arguments. That will future proof it for any other hierarchical model derivative that might use different names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants