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

Multimessenger Inference updates #262

Merged
merged 7 commits into from
Oct 10, 2023

Conversation

haukekoehn
Copy link
Contributor

Some small changes that previously obstructed multimessenger inference.

  • Use GenericCombineLightcurveModel to pass down the filters to the GRB model (previously the GRB didn't get any filters and tried to calculate the lightcurve for all 181)
  • bug fixes when calling parameter conversion
  • set DOI to permanent doi if DOI is not retrievable (see opened issue on github)
  • update prior file for Bu2022Ye. Before the prior bounds exceeded the grid, now the bounds from https://arxiv.org/abs/2307.11080 are adopted

Hauke Koehn and others added 6 commits October 2, 2023 09:35
Bu2022Ye is only developed on a specific grid. The current prior exceeds
the grid, thus it should be modified.
Additionally, if no DOI can be assigned, now it will be set to the
PERMANENTDOI by default.
Change priors for Bu2022Ye.prior
Previously, the GRB model did not get the filters passed down from the
sampler. This is now fixed, additionally the
GenericCombineLightCurveModel is introduced for the joint inference
likelihood.
@@ -369,7 +368,7 @@ def observation_angle_conversion(self, parameters):
def generate_lightcurve(self, sample_times, parameters):
if self.parameter_conversion:
new_parameters = parameters.copy()
new_parameters, _ = self.parameter_conversion(new_parameters, [])
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't the parameter_conversion for MMA takes not only the parameters dict but also the parameter keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For my setup self.parameter_conversion points to MultimessengerConversion.convert_to_multimessenger_parameters in joint/conversion.py line 468 and that function takes only the parameters as dict. i get an error when i leave the second argument there

@@ -2,7 +2,7 @@

import numpy as np

from ..em.model import SVDLightCurveModel, KilonovaGRBLightCurveModel
from ..em.model import SVDLightCurveModel, KilonovaGRBLightCurveModel, GRBLightCurveModel, GenericCombineLightCurveModel
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe remove the importing of KilonovaGRBLightCurveModel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@tsunhopang tsunhopang self-assigned this Oct 10, 2023
@tsunhopang tsunhopang added the bug Something isn't working label Oct 10, 2023
Copy link
Collaborator

@tsunhopang tsunhopang left a comment

Choose a reason for hiding this comment

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

LGTM

@sahiljhawar
Copy link
Member

@haukekoehn Ready to merge?

@tsunhopang tsunhopang merged commit 1b07393 into nuclear-multimessenger-astronomy:main Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants