SNEWS2 / snewpy

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

Make the `Detector` and `flux.Container` classes a public interface for v2.0 #339

Open Sheshuk opened 1 month ago

Sheshuk commented 1 month ago

Some time ago I introduced the object-oriented interfaces for manipulating

They are documented, though the documentation might need an update and improvement. For Detector, DetectionChannel and SmearingMatrix the docstrings are in place (see code) but they are not shown in the Sphinx docs yet.

They also have the extensive usage example notebooks: Detector_demo.ipynb, FluxContainer_demo.ipynb

These interfaces have always been marked with "Users should not use this" sign. The plan was to test them in a development team, but I don't know if anyone was using them (at least I didn't get any feedback or suggestions).

I use snewpy for the rate calculations for various detectors - not only the ones defined in the SNOwGLoBES, and as a user I find the standard interfaces with generate_* extremely restricting and unflexible and incapable of doing what I need.

So I suggest to

  1. Declare this interface public in v2.0
  2. Update the documentation - make the classes visible
  3. Add usage examples to the main section of the documentation
JostMigenda commented 1 month ago

A couple of points off the top of my head, in no particular order:

Sheshuk commented 1 month ago

and we’ve discussed changing this in v2.0. To what degree does that resolve your issues ? If there are use cases that still don’t work, we should discuss if we could cover them as part of the snewpy.snowglobes redesign.

Are you referring to #209? No, I don't think any update of these functions will be sufficient. The problem with the generate_* functions is that they pack too many steps in one. - even if they're updated, it still means that it does: 1) Initialize a model 2) Get the neutrino flux from the model 3) Apply flavor transformation 4) Integrate the flux in the required time&energy bins 5) Select the detector from SNOwGLoBES data 6) Calculate the number of events in this detector

And it does all of this under the hood. That means the user has no access to the neutrino flux, or flavor transformation, or the detector setup. There are many use cases, when user needs to examine and/or modify them - in between these steps.

I understand that having one function is intended to keep the interface simpler for the user. But it serves only a single use case: if the user needs neutrino interactions in the detector. Other use cases, like plotting the neutrino flux and getting the detector information requires a user to use other functions (SupernovaModel.get_flux, or examining the snowglobes_data), and these tasks are very common.

I think we’ve had a couple of PRs building upon those private APIs, right? And speaking at least for myself, my summer student and I have explored these APIs a bit, and have been quite happy with them

Thanks, that's nice to know! So they might be useful for others as well

Do you think these APIs will need any non-trivial changes to fit in with all the other changes planned for v2.0?

I think only these:

In other aspects these classes will remain the same (if I'm not missing anything).