SNEWS2 / snewpy

A Python package for working with supernova neutrinos
https://snewpy.readthedocs.io
BSD 3-Clause "New" or "Revised" License
26 stars 19 forks source link

Missing directions/masses in Walk and Tamborra models #231

Closed mcolomermolla closed 1 year ago

mcolomermolla commented 1 year ago

This PR will close issue #78.

For Tambora_2014, I am in touch with Irene, who told me that they did not save the theta and phi used in the three directions of the paper, so for now I can't provide all directions for all masses.

JostMigenda commented 1 year ago

Regarding tests: In test_01_registry.py and test_02_models.py, the model parameters for the Walk models need to be updated as well. Also, when adding these new files, we need to bump the release_version in model_files.yml for the Walk_2018 and Walk_2019 classes to 1.3 to ensure the model downloader finds the right files.

Slight problem: Until we’ve tagged v1.3, the model downloader won’t be able to download the files, so we can’t test that. (@sgriswol, am I missing something here?)

sgriswol commented 1 year ago

You're right @JostMigenda, the model downloader attempts to grab models files using a URL that includes a specific version tag (1.2 currently). So until these new model are available under a specific tag, they can't be downloaded. The downloader also needs the correct version to be set in snewpy.models.model_files.yml.

Slight problem: Until we’ve tagged v1.3, the model downloader won’t be able to download the files, so we can’t test that. (@sgriswol, am I missing something here?)

mcolomermolla commented 1 year ago

I took a quick look and noticed that the new parameter options for Walk_2018 and Walk_2019 were missing from the __new__ methods in a few places. Let me know if I can help get these integrated!

Thanks @sgriswol for your review. Your suggested changes have been added.

JostMigenda commented 1 year ago

I added some small fixes for the tests. The remaining test failures are due to the download issue mentioned above; that’s unfortunately impossible to fix within the scope of this PR.

However, when I manually copy the newly added model files to the cache directory (the model_path defined in snewpy/__init__.py) on my computer and then run the tests locally, they succeed.