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

Remove multiplication/division by 0.2 MeV in `generate_time_series` and `SimpleRate` #219

Open Sheshuk opened 1 year ago

Sheshuk commented 1 year ago

In the implementation of snowglobes_interface.SimpleRate I see that we're dividing by 0.2 MeV https://github.com/SNEWS2/snewpy/blob/b35379952fe18aedc28cdb106f1ee9e7ad08b082/python/snewpy/snowglobes_interface.py#L173-L175 because earlier we multiplied by this value in snowglobes.generate_time_series: https://github.com/SNEWS2/snewpy/blob/b35379952fe18aedc28cdb106f1ee9e7ad08b082/python/snewpy/snowglobes.py#L119-L122 and snowglobes.generate_fluence: https://github.com/SNEWS2/snewpy/blob/b35379952fe18aedc28cdb106f1ee9e7ad08b082/python/snewpy/snowglobes.py#L286-L288

If I understand correctly, this factor exists as an approximation of integral over the energy bin. The problem is that the energy bins can be different, so this hardcoded value is no longer valid in some cases (for example using smaller bins for presupernova flux). And this leads to physically invalid output of the generate_* for those cases - if anyone uses this output apart from feeding it to SimpleRate.

SimpleRate makes it's own approximation of the energy integration, which could be improved (in a separate issue), and doesn't use this

What I suggest

Remove this 0.2 MeV factor everywhere.

mcolomermolla commented 1 year ago

I fully agree, this is always a nightmare when comparing with your separate full simulation.

JostMigenda commented 1 year ago

Originally, the reason for picking these 0.2 MeV bins is that that was the default energy binning in the flux files included with SNOwGLoBES; so the generate_* functions were supposed to use the same binning in the files they write out. Now that we’ve moved to SimpleRate, I agree that we don’t need to stick with that binning.

That said, I would wait to remove this constant factor until we’re also updating the hardcoded energy binning; as you correctly say, updating one without the other would lead to invalid output if anyone uses generate_* without feeding the output to simulate.