Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Sheshuk opened this issue Oct 7, 2022 · 2 comments
Assignees
Labels
suggestion An idea that needs to be discussed/approved before starting implementaion
Milestone

Comments

@Sheshuk
Copy link
Contributor

Sheshuk commented Oct 7, 2022

In the implementation of snowglobes_interface.SimpleRate I see that we're dividing by 0.2 MeV

# Fluence (flux integrated over time bin) in cm^-2
# (must be divided by 0.2 MeV to compensate the multiplication in generate_time_series)
fluxs = np.interp(energies, flux[:, 0], flux[:, 1], left=0, right=0)/2e-4

because earlier we multiplied by this value in snowglobes.generate_time_series:
# Generate energy + number flux table.
for j, E in enumerate(energy):
for flavor in Flavor:
osc_fluence[flavor] = osc_spectra[flavor][j] * dt * 0.2 * MeV / (4.*np.pi*(d*1000*3.086e+18)**2)

and snowglobes.generate_fluence:
for j, E in enumerate(energy):
for flavor in Flavor:
osc_fluence[flavor] = osc_spectra[flavor][j] * dt * 0.2 * MeV / (4.*np.pi*(d*1000*3.086e+18)**2)

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.

@Sheshuk Sheshuk added the suggestion An idea that needs to be discussed/approved before starting implementaion label Oct 7, 2022
@Sheshuk Sheshuk self-assigned this Oct 7, 2022
@mcolomermolla
Copy link
Collaborator

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

@JostMigenda
Copy link
Member

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.

@JostMigenda JostMigenda added this to the v2.0 milestone Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion An idea that needs to be discussed/approved before starting implementaion
Projects
None yet
Development

No branches or pull requests

3 participants