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

Improvements to utils/neutrino_astronomy.py #236

Closed
mlincett opened this issue Dec 15, 2022 · 7 comments
Closed

Improvements to utils/neutrino_astronomy.py #236

mlincett opened this issue Dec 15, 2022 · 7 comments

Comments

@mlincett
Copy link
Collaborator

Some time ago, I had started a review of this module, in particular I would like to tidy up the "Total Flux" and "Flux from the nearest source" calculations.

In the following snippet:

# Energy requires a 1/(1+z) factor
zfactor = find_zfactor(lumdist)
etot = (si * area * e_integral).to(u.erg / u.s) * zfactor

the energy-integrated flux at Earth is multiplied by 1+z as a consequence of the energy range on which the integral takes place being redshifted w.r.t. the rest frame of the source.

From my calculations this should be (1+z)^(1-gamma) that for gamma = 2 should give 1/(1+z). Note that the comment mentions a 1/(1+z) factor while code multiplies by (1+z). For sure this should depend on EnergyPDF and gamma.

Probably my calculations should be double-checked as well, I am attaching here the file that @JannisNe and @robertdstein have already received by mail: nu_astro.pdf

@mlincett
Copy link
Collaborator Author

After a mail exchange with @robertdstein , probably the most convenient option is to remove the "flux from nearest source" calculation. This in light of the fact that:

  • the feature requires a revision in any case (at least it should depend on EnergyPDF);
  • it has no further use in flarestack and it is more of a convenience for the user;
  • calculating the nearest source only is kind of arbitrary (the nearest it's not necessarily the one with the strongest flux).

We can leave luminosity calculation as an "exercise to the reader" and simplify the results dictionary.

@JannisNe
Copy link
Collaborator

Hi sorry for the long wait! I went through it and I think you are right. In that case, your and Robert's suggestion sounds good!
Thanks for taking care!

@mlincett
Copy link
Collaborator Author

mlincett commented Jan 9, 2023

Thanks for the feedback @JannisNe and @robertdstein .

I drafted PR #242 with the idea of cleaning up the code and removing the controversial calculations.

Unfortunately, it seems that the field Mean Luminosity (erg/s) in the output was used in several past analyses:

flarestack/analyses/benchmarks/benchmark_blazar_flare_search.py:        e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/ztf/test_sens_dep.py:                e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/ztf/test_sensitivity_depth_time.py:                    e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/ztf/test_sensitivity_depth.py:                e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/tde/compare_fitting_weights.py:                e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/tde/calculate_jetted_model_limit.py:e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/tde/compare_spectral_indices.py:                        e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/tde/compare_cluster_search_to_time_integration.py:                e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/tde/compare_spectral_indices_individual.py:                e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/agn_cores/loop_over_catalogues.py:                        e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/ccsn/stasik_2017/calculate_sensitivity_reproduce_stasik_decay.py:                e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/ccsn/stasik_2017/calculate_sensitivity_reproduce_stasik.py:                e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/ccsn/necker_2019/decay_sensitivity.py:                e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/ccsn/necker_2019/box_sensitivity.py:                e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/fermi/gb6_fermi_blazar_flare_search.py:        e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/txs_0506_056/compare_txs_spectral_indices.py:                    e_key = "Mean Luminosity (erg/s)"
flarestack/analyses/txs_0506_056/INTRO1.py:print(astro_sens["Mean Luminosity (erg/s)"], "erg/s")
flarestack/core/unblinding.py:                        astro_dict["Mean Luminosity (erg/s)"] * inj_time

so I am not sure it is wise to discontinue it right away.

Please advise!

@JannisNe
Copy link
Collaborator

JannisNe commented Jan 9, 2023

In my opinion, we do not have to require past analyses to run with the most recent version of the code. They all mention their respective flarestack version with which we can reproduce them. So I'm in favour of getting rid of it.

@mlincett
Copy link
Collaborator Author

mlincett commented Jan 9, 2023

Alright! Will do the last cleanups and proceed with the merge.

@mlincett
Copy link
Collaborator Author

The removed functionality touches the calculate_upper_limits() in the core.unblinding module.

It seems though that calculate_upper_limits() is not covered by unit tests (see #243) nor it is invoked in any of the past analyses (at least the ones stored in the repository).

@mlincett
Copy link
Collaborator Author

The outputs based on "Mean Luminosity (erg/s)" have been removed from calculate_upper_limits().

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

No branches or pull requests

2 participants