removed sample_times arg in .generate_lightcurve definition#82
removed sample_times arg in .generate_lightcurve definition#82tylerbarna wants to merge 276 commits intonuclear-multimessenger-astronomy:mainfrom
Conversation
…constrain_R14_trend Add documentation to constrain combined EoS analysis
…/main typing mistake correction
…/main direction of the file gw_posteriors.txt
…rma049/inst Instruction file merged
numpy warning messages
…/lanl This PR adds LANL models.
…/main Update of ZTF and Rubin light curves detection files
changed behaviour of models doing .generate_lightcurve so they just define sample_times by retrieving it from self.sample_times. So any calls to the models are unaltered. I tried to find all calls to .generate_lightcurve and correct them, but I haven't validated all use cases.
|
@tylerbarna This is a good start. I think to have the best of both worlds, why not allow generate_lightcurves to take an optional sample_times argument? So if you pass it, great, and if not, it defaults to self.sample_times? |
That's a good idea, my original thought was to have it so you only had to provide sample_times to generate_lighcurve and/or generate_spectra, but I wasn't sure if other parts of the code outside of models.py require model objects to have sample_times. There were only a handful of generate_* calls in comparison |
|
@tylerbarna Yeah I am not unsure. I think to be safest you can for now just make it an argument that defaults to None and allows you to pass it for backwards compatability. |
can provide sample_times when calling generate_lightcurve but is not required. May still have backward compatibility issues if code that calls generate_lightcurve doesn't have sample_times as a keyed parameter (ie generate_lightcurve(sample_times=sample_times, parameters=parameters) would work, but generate_lightcurve(sample_times, parameters) would break since sample_times is no longer the first parameter)
|
made the argument optional, but any code that calls generate_lightcurve without passing the sample times as a keyed argument will still break since the original version has sample_times as the first parameter |
|
@tylerbarna For now, you can still pass samples_times=sample_times in analysis.py and other places to help mitigate behavior changing right? |
|
yes, you should be able to, and looking through analysis.py, most calls to model objects reference the keys already |
mcoughlin
left a comment
mcoughlin
left a comment
There was a problem hiding this comment.
@tylerbarna A few places where I think you can still pass: sample_times=sample_times.
| # Generate the lightcurve | ||
| ######################### | ||
| _, mag = light_curve_model.generate_lightcurve(sample_times, bestfit_params) | ||
| _, mag = light_curve_model.generate_lightcurve(parameters=bestfit_params) |
There was a problem hiding this comment.
I guess I am thinking of here. shouldn't there be a sample_times=sample_times
| if len(models) > 1: | ||
| _, mag_all = light_curve_model.generate_lightcurve( | ||
| sample_times, bestfit_params, return_all=True | ||
| prameters=bestfit_params, return_all=True |
11 participants
changed behaviour of models doing .generate_lightcurve so they just define sample_times by retrieving it from self.sample_times. So any calls to the models are unaltered. I tried to find all calls to .generate_lightcurve and correct them, but I haven't validated all use cases.
resolves #81