Closed Theodlz closed 1 year ago
Just wanted to check in on this @Theodlz - would you be in favor of adding nlive
, error_budget
and Ebv_max
to default_analysis_parameters`?
It sounds like a good idea @bfhealy. Best would be to still have default values here, that are picked if not specified by skyportal for any reason.
@Theodlz I'm getting the following error when using the Me2017
model with the API service:
2023-07-12 09:34:06 nmma: Exception while running the model: 'log10_mej'
[09:34:06 nmma] Exception while running the model: 'log10_mej'
2023-07-12 09:34:06 nmma: Traceback (most recent call last):
File "api/app.py", line 214, in run_nmma_model
main(args=args)
File "/Users/bhealy/nmma/nmma/em/analysis.py", line 579, in main
result = bilby.run_sampler(
File "/Users/bhealy/miniforge3/envs/nmma_env/lib/python3.8/site-packages/bilby/core/sampler/__init__.py", line 190, in run_sampler
sampler = sampler_class(
File "/Users/bhealy/miniforge3/envs/nmma_env/lib/python3.8/site-packages/bilby/core/sampler/pymultinest.py", line 86, in __init__
super(Pymultinest, self).__init__(
File "/Users/bhealy/miniforge3/envs/nmma_env/lib/python3.8/site-packages/bilby/core/sampler/base_sampler.py", line 900, in __init__
super(_TemporaryFileSamplerMixin, self).__init__(**kwargs)
File "/Users/bhealy/miniforge3/envs/nmma_env/lib/python3.8/site-packages/bilby/core/sampler/base_sampler.py", line 251, in __init__
self._verify_parameters()
File "/Users/bhealy/miniforge3/envs/nmma_env/lib/python3.8/site-packages/bilby/core/sampler/base_sampler.py", line 419, in _verify_parameters
self.log_likelihood(theta)
File "/Users/bhealy/miniforge3/envs/nmma_env/lib/python3.8/site-packages/bilby/core/sampler/base_sampler.py", line 841, in log_likelihood
return Sampler.log_likelihood(self, theta)
File "/Users/bhealy/miniforge3/envs/nmma_env/lib/python3.8/site-packages/bilby/core/sampler/base_sampler.py", line 534, in log_likelihood
return self.likelihood.log_likelihood()
File "/Users/bhealy/nmma/nmma/em/likelihood.py", line 108, in log_likelihood
lbol, mag_abs = self.light_curve_model.generate_lightcurve(
File "/Users/bhealy/nmma/nmma/em/model.py", line 865, in generate_lightcurve
param_dict[key] = new_parameters[key]
KeyError: 'log10_mej'
I get the same error when running light_curve_analysis
directly:
light_curve_analysis --model Me2017 --interpolation_type tensorflow --outdir outdir_me_ps1 --label injection --prior priors/Me2017.prior --tmin 0.5 --tmax 20 --dt 0.5 --error-budget 1 --nlive 512 --Ebv-max 0 --injection ./injection.json --injection-num 1 --injection-outfile outdir_ps1_2048/lc.csv --generation-seed 42 --plot --remove-nondetections --sampler pymultinest --filters ps1__g,ps1__r,ps1__i,ps1__z,ps1__y
Creating light curve model for inference
Creating injection light curve model
Traceback (most recent call last):
File "/Users/bhealy/miniforge3/envs/nmma_env/bin/light_curve_analysis", line 33, in <module>
sys.exit(load_entry_point('nmma==0.0.13', 'console_scripts', 'light_curve_analysis')())
File "/Users/bhealy/nmma/nmma/em/analysis.py", line 414, in main
data = create_light_curve_data(
File "/Users/bhealy/nmma/nmma/em/injection.py", line 114, in create_light_curve_data
lbol, mag = light_curve_model.generate_lightcurve(
File "/Users/bhealy/nmma/nmma/em/model.py", line 865, in generate_lightcurve
param_dict[key] = new_parameters[key]
KeyError: 'log10_mej'
This doesn't seem to be specific to the API, and the other models listed in my local SkyPortal (Piro2021
, nugent-hyper
, TrPi2018
, Bu2022Ye
) run well via API.
Hi @bfhealy, so my understanding is that the generate_lightcurve
function takes in a parameters dictionary, and also has a list of what parameters are to be expected. And it simply can't find this one. What I don't quite get is how it works for other models and not this one (some even have the same parameters.
There's a also a method that creates these parameters passed to generate_lightcurve
based on what the args we give to the sampler.
I'll keep looking
@Theodlz I think the issue with Me2017 has to do with the case of a parameter name. Interacting with the variables right before the error in L865 of model.py
(param_dict[key] = new_parameters[key]
) gives these results:
>>> parameters
{'luminosity_distance': 104.43457029096375, 'beta': 1.7482626166209436, 'log10_kappa_r': 1.981785973554159, 'KNtimeshift': -1.4635586137596275, 'log10_vej': -0.6056705156035385, 'log10_Mej': -2.518467264290937, 'Ebv': 0.22363411947788597}
>>> self.model_parameters
['log10_mej', 'log10_vej', 'beta', 'log10_kappa_r']
Somehow, the 'm' in log10_mej
has been capitalized in parameters
, raising the aforementioned KeyError
. I'm not yet sure where this capitalized parameter comes from.
@bfhealy Aren't they coming from whatever you are posting the analysis from?
@mcoughlin @Theodlz Yes, this was the issue! Last month I copied my priors
directory to the api
directory since I wasn't using docker. Since that time the Me2017 prior was updated to have the correct name for log10_mej
, and when I update my api/priors
directory the API works as intended.
@bfhealy @Theodlz should be good to merge when ready then.
@Theodlz @mcoughlin I do see one more issue to consider, but I think it may necessitate a SkyPortal change rather than a change to this PR. It seems skyportal names its ZTF filters g
, r
, and i
, but newer models like Bu2022Ye
use ztfg
, ztfr
, and ztfi
.
Upon removing my api/svdmodels
directory that had old trained models in it, the code attempts to use the model on Zenodo as intended. I then get the following error:
Traceback (most recent call last):
File "api/app.py", line 216, in run_nmma_model
main(args=parser.parse_args(args))
File "/Users/bhealy/nmma/nmma/em/analysis.py", line 549, in main
model_names, models, light_curve_model = create_light_curve_model_from_args(
File "/Users/bhealy/nmma/nmma/em/model.py", line 971, in create_light_curve_model_from_args
lc_model = SVDLightCurveModel(**lc_kwargs)
File "/Users/bhealy/nmma/nmma/em/model.py", line 251, in __init__
_, model_filters = get_model(
File "/Users/bhealy/nmma/nmma/utils/models.py", line 265, in get_model
raise ValueError(
ValueError: models list from zenodo does not have filters g,r for Bu2022Ye_tf
@bfhealy That should be easy enough on the SP side.
@bfhealy I think one thing that would be nice in a future PR is adding to the docs how to set this up for use with SkyPortal.
@mcoughlin Agreed, I'll make a note of that.
Re-opening #99 on a clean branch