Interface preSN models into snewpy

[joesmolsky]

I would like to add preSN models (https://github.com/SNEWS2/presn-models) to the snewpy models. I would also like to run time series through SNOwGLoBES for varying start times before collapse.

I am willing to do the work, but don't know the best format for the models' spectra and times. Is there anyone that would like to help me add these features to snewpy?

[Sheshuk]

Hi @joesmolsky , this is something I was starting to work on for some time now, thank you for getting to it! I've implemented a reader class here: https://github.com/SNEWS2/presn-models/blob/master/reader/reader.py but it needs to be adapted for snewpy base class.

[Sheshuk]

We need to think about models bookkeeping though. Currently all the ccSN models are inside the snewpy.models module, we don't want to mix them with presupernova ones

[jpkneller]

You can build a preSN model base class along the lines of SupernovaModel and then write specialized classes for the different preSN data sets. If the base class has the same methods as SupernovaModel then you can re-use a lot of the other parts of the SNEWPY code e.g. generate_fluence. We could also re-organize the models folder into CCSN, Type Ia, PISN and preSN.

[Sheshuk]

We could also re-organize the models folder into CCSN, Type Ia, PISN and preSN.

So should we move current models to a CCSN submodule? They can be accessed via snewpy.models.CCSN.ModelName instead of snewpy.models.ModelName. This is some change of the interface, but probably not critical. Also I see that most of the models share the same code for get_initial_spectra (like here, with only a very slight differences in the checks after. This asks for creation of a parent class with this functionality. Should I make something like PinchedModel base class? This would help to make sanity checks like in #120 more consistent, as they would be applied to all subclassed models.

[jpkneller]

The CCSN module is a good idea. As you said, the changes are not critical. A PinchedModel base class could also work if you use multiple inheritance i.e. define the abstract base class SupernovaModel, then define PinchedModel, and have, say OConnor_2013 inherit from both. It would save a lot of lines of repetitive code.

[Sheshuk]

The models interfaces are in place. Next step is to test them vs. the generate_fluence/generate_time_series methods. Also I need to add unit tests.

[Sheshuk]

Looks like the standard generate_fluence/generate_time_series aren't suitable for presupernovas: they're using equidistant time bins for generation, which doesn't make much sense in our case. So I'll have to make a new function, receiving an array of timestamps. And keep the old functions as interface to the new one.

[evanoconnor]

I'm pretty sure that the tstart/tend option for generate_fluence allows for this flexibility? I have never used anything other than non-equidistance bins, but it is coded for an arbitrary array of tstarts and tends. The idea is it integrates the model between tstart[i] and tend[i]

[Sheshuk]

You're right, I didn't look at the fluence parameters. Still, the generate_time_series doesn't allow to do this. I can make a series of calls to generate_fluence, but that's takes ridiculously long time.