Skip to content

Update snowglobes.py - Experimental - #441

Draft
jpkneller wants to merge 162 commits into
mainfrom
jpkneller-new-generate
Draft

jpkneller wants to merge 162 commits into
mainfrom
jpkneller-new-generate

Conversation

@jpkneller

@jpkneller jpkneller commented Jun 22, 2026

Copy link
Copy Markdown
Contributor
  • Single generate function accepts a model class (not a string) and input times and energies. Returns Container object of flux or fluences. generate_time_series and generate_fluence are retained but are deprecated.
  • New function called calculate combines action of simulate and collate. The input is a flux or fluence Container (i.e. return object from generate) or a string with the name of the file written by generate. Only calculates 'smeared' or 'unsmeared' as given by 'detector_effects' argument, not both. If string is input, a numpy archive file is made and the return is the name of the file: if not then then the returns is a nested dictionary (detector : channel) of event rates / numbers.

@jpkneller jpkneller changed the title Update snowglobes.py Update snowglobes.py - Experimental Jul 7, 2026
@jpkneller

Copy link
Copy Markdown
Contributor Author

... it looks like you’re still working on it actively (with about 30 commits since you requested the review); so please let me know when it settles down and is ready for a thorough review.

The integration tests etc. and notebooks were giving me a lot of errors. I ended up having to touch a lot of files for small little edits.

In the meantime, my first, high-level thoughts are:

* We haven’t marked the `generate_*` functions as deprecated in any earlier snewpy release, so dropping them without warning would cause serious headaches for users. I’d prefer adding `generate` as a new function; we might be able to turn (one of) the old functions into a thin layer on top of the new implementation, to avoid having too much duplicate code.

Fair point. In the new version I retained the old *generate_ ** functions with an added warning that they are deprecated. One minor thing to note is that the new generate function does not integrate over the energy bins as the older *generate_ ** functions used to. In practice this should not cause a problem: the Container has a method called integrate_or_sum which will undertake the appropriate action for the circumstances, and by avoiding the energy bin integration in generate, the spectral fluences can be re-sampled by e,g, the RateCalculator if needed.

* I have similar worries about the other functions—making these changes without giving any warning by deprecating arguments first would break all existing code.

* Deprecating arguments in-place might work (but require some careful thought!); though before we go to that effort, I wonder whether it’s even worth preserving the three-step generate/simulate/collate structure of the API? In particular, the separation between `simulate` and `collate` comes from an era when `simulate` was done by calling the SNOwGLoBES binary and a separate step was necessary to read in the output files generated by SNOwGLoBES and make them available in snewpy. Now that collate is a fairly light post-processing step, maybe we could include it with the calculation step?

The old versions of simulate and collate are also retained. The new function to replace them is called calculate and a) combines the effect of simulate and collate and b) returns Container objects.

In a separate PR, I added a collate function to rate_calculator.py which might be useful.

The detector_effects argument is now used - previously it was ignored.

Yep; this was left over from early snewpy versions, where we depended on (SNOw)GLoBES and transitioned to our internal rate calculation. We decided in #193 that it is no longer needed, since the performance overhead of calculating both unsmeared and smeared rates became sufficiently small. Unfortunately, it looks like we forgot to mark it as deprecated at the time. 🤦

I am open to having calculate add another level to the nested dictionary of event rate tables it returns, but is this really useful?

@jpkneller
jpkneller marked this pull request as draft July 9, 2026 21:40
@jpkneller

Copy link
Copy Markdown
Contributor Author

I am going to keep this branch for reference but I am leaning more and more towards ditching this effort. If #445 is accepted, everything we need to easily replace the snowglobes module is available.

@jpkneller jpkneller added the reference Kept for reference only, not meant to be merged. Usually used for experimental branches label Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reference Kept for reference only, not meant to be merged. Usually used for experimental branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants