Open JostMigenda opened 1 year ago
We discussed this at the SNEWS telecon just now. A quick summary:
SupernovaModel
and FlavorTransformation
, there was general agreement that those should be initialised by the user and then passed as objects rather than strings.
__str__
method is probably enough; or we may need to add a short method for that.generate_fluence
function, which is physically more accurate. Longer computation time was an issue in the past; but the implementation has changed somewhat since, so we should double check if that is still a concern.I like Andrey's suggestion for replacing the model_type and the transformationtype strings with class instances. With the current version of SNEWPY, flavor transformations which require additional parameters beyond the three vacuum mixing angles cannot be used unless the user dives into the generate functions. The two strings are included in the output filenames made by generate_ so we could modify both the flavor transformation and model classes by adding to each a to_tex() method which returns a string with the model or flavor transformation name. This is what is done in the Flavor class. I would also like to suggest we deprecate the generate_time_series function and move to only having a generate_fluence function. For the time being, generate_time_series can be modified so that it creates the list of tstart and tend that then gets passed to generate_fluence.
Commenting on the second point: I am in favor of keeping only one function to make users life easier, and use generate_fluence. Indeed, I remember it was taking long when using long duration and small binning simulations, but with the better performance after the updates and most of the models not being long, it is affordable. I can provide a computational time if you're interested. It is also true it is more accurate for simulating a lightcurve with "fine" binning.
I think this should be planned for SNEWPY v2.0, so I'll change the milestone
There are a couple of problems with the
generate_fluence
andgenerate_time_series
functions insnewpy.snowglobes
:generate_time_series
takes a number of bins or a bin duration, whereasgenerate_fluence
takeststart
andtend
which can be either a single value (corresponding to 1 bin) or a list of times (to create multiple bins of arbitrary lengths). As we recently saw on Slack, this even trips up us main developers sometimes.These issues are in principle independent; but they may all require backwards-incompatible changes to resolve, so I think we should try to tackle them in a coordinated fashion to minimise disruption. I have a rough idea how to do that, but want to coordinate with people working on the PRs mentioned above first.
¹ There is an optional
snmodel_dict
argument, which we could use to fit in all the physics parameters; but that would be quite awkward to use (and we’d still have the (then unused) file path as a required argument).