Skip to content

Conversation

@sybenzvi
Copy link
Contributor

This PR adds software support for new 3D models posted in https://www.astro.princeton.edu/~burrows/nu-emissions.3d.update/.

The model files have been uploaded to the snewpy-models-ccsn repository. Complete the following activities before merging this PR:

  • Add unit tests of the new Fornax_2024 class and models.
  • Add a documentation notebook demonstrating the output.
  • Make a new tag/release of the snewpy-models-ccsn project.
  • Update snewpy/models/model_files.yml so that all CCSN models point to the new release repo.

Comment on lines +433 to +440
_fornax_2024_progenitors = [
'u8.1', '9a', '9b', '9.25', '9.5',
'z9.6', '11', '12.25', '14', '15.01',
'16.5', '16', '17', '18', '18.5',
'19', '19.56', '20', '21.68', '23',
'24', '25', '40', '60', '100']

_fornax_2024_masses = [float(re.sub('[A-Za-z]', '', p)) for p in _fornax_2024_progenitors] << u.Msun
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will map both 9a and 9b progenitors to 9.0 * u.Msun; so the latter would override the former when creating the _mass_to_progenitor dict later and we wouldn’t be able to access the 9a progenitor.

The two both appear to be based on the 9 Msol progenitor from Sukhbold et al. 2016; with the caption of fig. 7 in arXiv:2401.06840 explaining:

The only difference between models 9(a) and 9(b) is the imposition of slight velocity perturbations upon infall in the former (Wang & Burrows 2023a).

Since none of the other progenitors have these velocity perturbations, do you think we should just drop the 9a progenitor and document that 9 Msol corresponds to 9b? Or is it worth the small extra effort to support both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A late answer to this question: I think it's OK to drop the 9a progenitor, justified by the fact that the velocity perturbations are not present in any other model.

Your proposal is just to remove '9a' from the progenitors list?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants