Models bookkeeping

[Sheshuk]

Hi all, I thought this "discussion" functionality can be useful, to put the ideas of possible improvements, without spamming in the issues.

Some thoughts on the models bookkeeping, while working on #121 : the presn models datafiles are quite heavy: 311M, so in addition to the existing snewpy/models ~330M the repository becomes quite unwieldy. I see here two solutions:

1. Keep presn datafiles as a separate git submodule, so they're not downloaded by default, unless git submodules init is called. Preferably we do the same with ccsn datafiles (and delete them from git history of this package).
2. Move the models to a separate package or packages, installable by pip. We can use flexible system of the extra_requires in the setup to put these models
   We already have the get_models function, so any internal change will be wrapped by it and invisible to the final user. But it might make running our tests faster.

Any input is appreciated

[JostMigenda]

One important thing to keep in mind: For every release of SNEWPY, data files and model code must be in sync. If those are in one repo, we get that effectively for free; if they aren't, we'll have to find a way to sync them up as part of the release/publish to PyPI workflow.

[JostMigenda]

We don’t need to, per se. But if we don’t (and only tag a new version of snewpy_models when there are changes to the models and to the corresponding code in snewpy.models), that’s much harder to automate; I think it would require more manual steps during the release/publish process for a new SNEWPY version, which in turn increases the risk of human error.

Edited to add:

• I don’t see a downside to publishing new snewpy_models versions more often than needed. Yes, it’s an additional dependency to download, but the package itself would just be a few KB (URL list plus the get_models() function).
• Semantic versioning doesn’t make too much sense for snewpy_models, so we’re not losing information by following SNEWPY’s version numbering. And on the contrary, I think there’s some value to having the version number of snewpy_models always match the snewpy version number, if only for simplicity.

[Sheshuk]

I don’t see a downside to publishing new snewpy_models versions more often than needed. Yes, it’s an additional dependency to download, but the package itself would just be a few KB (URL list plus the get_models() function).

Okay, then the simplest way is to keep the urls list and get_models in the main repository (as it is now), but put the model data files elsewhere.

Zeondo looks like a good place for the data files to me. We can store the models together with all needed metadata, and also make sure the user has the needed citation information.
Some datasets are already present there, for the others we can contact the authors and ask them (to give us permission) to put it in there.
They have a REST API we can use for downloading, although it requires an access token. Download links also work, but have more traffic limitations.

[JostMigenda]

The authors have already given us permission to distribute their data sets with SNEWPY (and in fact, we already upload every new SNEWPY release—including model files—to Zenodo), so I don’t think that would be an issue.

Could you describe what the workflow to add a new model and publish an updated SNEWPY version would be in your proposal? I think it may be useful to see what steps are required (and which of them are manual vs. automated). For comparison, here’s the current workflow:

1. Manually commit model files (and appropriate SupernovaModel subclass, if necessary) to the SNEWPY repo.
2. Manually tag a new version.
3. A GitHub action will automatically generate the list of current URLs for all files included in that version and publish the new SNEWPY version to PyPI.

[Sheshuk]

I see it as list of Model names vs. Zenodo links. So to add a new model would require:

1. Upload/find the data files on Zenodo
2. Implement a SupernovaModel class reading the file
3. Add the link in the list

This models list can be some sort of a simple database, containing model name, zenodo link and metadata for that link.
This list is kept within the package and gets automatically versioned with it, so nothing special is required for the PyPI

[JostMigenda]

We’ve had several cases recently where data files needed to be added (#108) or modified (#168, #161). This was something I had not anticipated when originally writing the get_model() function (which is why it currently skips any files that were previously downloaded); but now that we know it does, I wonder how that would work on Zenodo.

Is there e.g. a way to give all members of a GitHub organisation access to edit Zenodo data sets?
Without that, the two options I could see are

• sending the original uploader of the data set a patch file and asking them to update the data set, or
• re-uploading the whole data set with the modifications.
  Both seem significantly more complicated than the current process. They would also make it significantly more complicated to diff the data files to see what has changed.

[Sheshuk]

Another thing to consider: since the models are downloaded by get_models, we might as well just have a fixed (wrt installation path) location for the model files.
This will free the user from pointing to the models file, which is actually quite cumbersome. To have Bollig_2016 model, I have to print the model's name several times:

from snewpy.models import Bollig_2016
model = Bollig_2016('models/Bollig_2016/s11.2c')

while it's clear that when trying to create Bollig_2016 model I need the datafile. So, instead I would pass the desired metadata - in this case, the progenitor mass.

model = Bollig_2016(progenitor_mass='11.2')

and this would check the if the models manager has this model downloaded (and download it if needed).

[JostMigenda]

I’m strongly opposed to trying to hide the file system location of these model files from users. In addition to using additional models that aren’t part of SNEWPY, users may also want to use SNEWPY model files with other software (like sntools or completely custom scripts) that doesn’t know about SNEWPY’s model manager.

File system paths also have the advantage that they can be autocompleted; while arbitrary argument values (that get stitched together into a file name later) cannot.
It would also be a bad experience for users if each model requires a different set of parameters (all require mass, but many require additional parameters which may include arbitrary combinations of metallicity and/or EOS and/or some custom mixing length parameter and/or rotation and/or …).

If you’re opposed to writing the model name more often than absolutely necessary, an alternative idea might be something like:

from snewpy.models import Model
m = Model('models/Bollig_2016/s11.2c')

where Model could contain code to determine the format based on the file system path—look for the pattern Name_YYYY and see if there’s a matching SupernovaModel subclass.

[Sheshuk]

Thank you for your input! So we have here three use cases:

1. Using standard models, provided with the snewpy. In that case user doesn't need to handle the data files.
2. Using custom/modified data files. In that case user needs the model class to read the formatted data file, but doesn't need to know the location of the standard data files.
3. Using model data files with the external tools. User needs to know where the data files are.

Of course we need to keep the ability to read the custom provided files. But, strictly speaking, reading the custom file with OConnor_2015 model doesn't give you OConnor_2015, but something different, so the should can be organized by the data file formats, not the models. In other words the specific Name_YEAR model implies using one specific dataset, and doesn't make sense otherwise.

It would also be a bad experience for users if each model requires a different set of parameters

Look at the models' constructors we have now. That's exactly the case, and my bad user experience lead me to start this discussion 😃 Before instantiating the model I need to know:

• data files location
• what format for the data path is expected. A full file name? Directory? File prefix?
• metadata: EOS, progenitor mass etc. Sometimes it's used for the file name generation (like for Garching format) so you need to know what options are present. Sometimes it's just stored in the class and printed nicely, but is completely independent from the data.

Having some manager with these locations would spare the user of the first two points.
As for the tab autocompletion - there are ways to customize it for the class using __dir__ , but I'm not sure yet how to make it to list allowed function arguments. Need to think about it.

[Sheshuk]

One simpler idea is to use type hints for the input parameters. In particular, typing.Literal allows to list the allowed values, and this will be clearly seen, although not autocompleted. Example:

from typing import Literal

def  read_model(mass: Literal[11.2, 27.0], eos: Literal['LS220','SFHo']):
        self.data = read_file(f'{path}/MyModel_{mass}_{eos}.dat')

The signature is visible in Jupyter if you type MyModel(<Shift+Tab>

[JostMigenda]

Oh, I didn’t know about typing.Literal! This looks useful in principle, but I see a few potential issues:

• It may be overwhelming in the case of the Warren_2020 model family (with 200 different masses)
• What if not all combinations of mass/eos/… are available? (E.g. for Nakazato_2013, the 30 Msol progenitor is available for several different EOS, while other progenitors aren’t.)
• It’s a bit more work for us to write down all the possible values of all parameters in the code
• Having the same information in multiple places (on the file system and in the code) always introduces a risk that they’ll get out of sync; e.g. if we add files to a model later (like in Add a new BH simulation with Togashi EOS #108). We already have this problem for each model with the README file, the demo notebook and the docstring of the respective SupernovaModel subclass.

One other thing to keep in mind is that that’s new in Python 3.8 so we’d need to update our version requirements accordingly. (Though requiring 3.8 doesn’t seem too agressive.)

[Sheshuk]

Yes, you're right, I didn't think about the possible combinations.

Okay, then there is another way: one can use IPython.set_custom_completer to add an autocompleting function. The function receives the previous text before cursor and should return the list of completed variants. It should not be hard to parse the parameters and suggest variants, based on the ones already entered.
I checked: setting this function this works for me in Jupyter notebook, the custom completion variants are shown first in the list, and then default completion is listed afterwards. So, this autocompleter can be enabled automatically in models.__init__ if the user runs with IPython, and will only work when user types something like

m = snewpy.models.MyModel(mass=10, eos=<Tab>

the manager would search the variants for MyModel with mass=10 and suggest the autocompletion.
Although I didn't make a reasonable parser function yet, just a stub, need to play a bit more with regex.

[Sheshuk]

Links on removing the large files:

• github
• stackoverflow
  So it should be possible.

[Sheshuk]

I think people intentionally downloading an old version is probably rare enough that we can neglect it

This would happen if one makes a package depending on snewpy, or just store the environment via pip freeze in a requirements.txt file to reproduce the environment later. Usually it makes sense to fix the package version you use, to avoid dependency confusion attack and in general keep things stable.
But anyway, those cases can be solved with post-release, and as for the people with already installed SNEWPY - I think it's enough just to put "PLEASE UPDATE SNEWPY" in the main branch README and in the ReadTheDocs, because this is where one would go searching for troubleshooting.
We can even update the ReadTheDocs for previous tags.

[sybenzvi]

@Sheshuk and @JostMigenda, shall we create a Zenodo link to one of the models directly "controlled" by SNEWS (presumably one of @evanoconnor's) and test the various cases?

We should also start contacting authors to see who is and is not alright with us making models available through this channel. While I do like the Zenodo DOIs, I'm concerned that citable DOIs have the potential to steal citations from papers and anger folks in the modeling community. The alternative would be to identify a host that does not provide DOIs.

[Sheshuk]

We have one of the preSN models: https://zenodo.org/record/2626645, so this might be good enough for tests.
As for using proper DOIs - we have the freedom to put the correct references in the models classes metadata, so they're shown when you print the model along with other parameters.
Optionally we can have a specific log message on shutdown, listing the references only for the used models:

If you use the calculations for the publication, please cite:
* Model2: [ref2]
* Model5: [ref5]

And finally, it's possible just to add "Please cite as [myref]" in the zenodo Readme file. That should work for the people who directly download the dataset without snewpy

[sybenzvi]

OK, want to make a zenodo-access branch of SNEWPY so we can test this mode?

[Sheshuk]

Yes, I'll work on that