Skip to content

Conversation

@jpkneller
Copy link
Contributor

Moved the SNOwGLoBES class and Analytic3Species class from ccsn.py into base.py.

jpkneller added 4 commits July 9, 2025 09:11
Moved the SNOwGLoBES class and Analytic3Species class from ccsn.py into base.py.
Moved SNOwGLoBES and Analytic3Species model classes from ccsn.py into base.py. This is a better match to the model hierarchy.
Update source of model class
Update source of model class
@jpkneller
Copy link
Contributor Author

First steps to get the Type Ia and PISN models at the same level as the CCSN

@jpkneller
Copy link
Contributor Author

The Integration Test errors are in the ipynb files which still import Analytic3Species from ccsn.py

Add PISN and TypeIa repository
@JostMigenda JostMigenda mentioned this pull request Aug 1, 2025
5 tasks
@jpkneller jpkneller changed the title Update ccsn.py Move model classes from ccsn.py to base.py Sep 2, 2025
@JostMigenda
Copy link
Member

Moving the Type Ia and PISN models out of snewpy.models.ccsn makes a lot of sense to me. I don’t like moving them to snewpy.models.base though—that file’s existence is an implementation detail which makes sense for our code organisation but not as part of the public API for users. From a user perspective, I think something like snewpy.models.pisn/snewpy.models.type1a would be clearer; and while it’s unlikely we’d get many more models in each of those modules, I think that’s okay.

On the other hand, the Analytic3Species class is a ccsn model; isn’t it? So I’m not sure that needs to be moved at all?


There’s also the issue of those three models behaving differently from all our other models. (The Ia/PISN models because we only have the integrated fluence, without time information; the Analytic3Species model because we don’t have any model files available to download.) But I’m not 100% sure how/whether to reflect that in the module structure.

@jpkneller
Copy link
Contributor Author

I added the pisn.py and pisn_loaders.py to the models folder with a Wright_2017 class, and published the PISN/Type Ia repo for the model data

@JostMigenda
Copy link
Member

I’m a bit confused—it looks like the new PISN and Type_Ia classes are now in #415 & #416, but those inherit from SupernovaModel and are independent of the SNOwGLoBES class.

Are those two PRs intended as a replacement for this PR here? Or should we combine them somehow?

@jpkneller
Copy link
Contributor Author

Yes they now inherit SupernovaModel so function like any other model class. We can now delete the SNOwGLoBES class.

@JostMigenda
Copy link
Member

Okay, I understand; thanks!

they now inherit SupernovaModel so function like any other model class

There are some important differences, though; right? IIRC, these models only contained fluences, so we have no time-dependent flux information? And I think that will also limit (at least in principle), which flavour transformations we can apply to those models, since transformations can be time-dependent.

Are there any other restrictions on flavour transformations? E.g., do all the in_sn transformations make sense to apply here? I vaguely remember something about those fluences having transformations baked in; but I see in the code that there are separate NoOsc files and we’re currently only using those.

@jpkneller
Copy link
Contributor Author

All the differences between the Type Ia and PISN models with the CCSN models have now been removed. The raw data files for the PISN and Type Ia contain time snapshots and as you noticed, they also contained the unoscillated spectra. By using the NoOsc data it is possible for the user to apply any of the in_sn transformations - the user would not be 'double applying'. It is true that applying some of the flavor prescriptions may not make sense physically but the onus is on the user to know why of the that is.

@jpkneller
Copy link
Contributor Author

BTW, when I ran the new Type Ia Jupyter notebook I had trouble downloading the data from the repo. @JostMigenda, can you verify.

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