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

Add a check for CMB field before deactivating the the redshift evolution #255

Merged
merged 2 commits into from
Dec 6, 2019

Conversation

lukasmerten
Copy link
Member

Should resolve #254
@Froehliche-Kernschmelze have you encountered this?

@lukasmerten lukasmerten requested a review from avvliet November 18, 2019 14:08
@MHoerbe
Copy link
Contributor

MHoerbe commented Nov 19, 2019

Yes, I have encountered this a while ago and this is clearly a bug and the fix is indeed as easy as suggested. The effect of this bug is that all redshift-dependence is switched off automatically regardless of the selected photon field. For the CMB this was obviously without consequences, yet for the IRB_Kneiske04 only the spectral shape at z=0 is used (see tabulated files for a comparison). Any other photon field that is not the CMB and IRB_Kneiske04, including its redshift dependence, is not taken into account anyways by the PhotoPionProduction as pointed out in #220

@lukasmerten
Copy link
Member Author

When I understand you correctly, @Froehliche-Kernschmelze, this will be fixed with #220. However, it won't hurt your pull request if we fix it -- at least for the Kneiske_04 field -- now?

Has anyone an example using redshift evolution in PPP, which can test with and without the fix. So we can estimate the influence of this. If not I'll try to find some time in the next days to look into this.

@avvliet
Copy link
Member

avvliet commented Nov 19, 2019

@lukasmerten I would indeed suggest first testing what happens with the fix before merging this pull request. The fix looks perfectly fine of course, but as the redshift evolution in PPP has basically been disabled before this fix it should be tested properly before this merge enables it.

@rafaelab
Copy link
Member

Hi all,
I think the commit is fine and this is a plain "bug" that hasn't been really affecting anything.
However, I agree with Arjen that it is more prudent to make sure nothing breaks (although this is unlikely).

Copy link
Member

@avvliet avvliet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the tests @lukasmerten performed I believe this is a good fix of the problem. It enables the 2D tabulated redshift dependence for photopion production, as a replacement for the global scaling with redshift. It looks like this 2D tabulated redshift dependence is working well.

@lukasmerten
Copy link
Member Author

Hi all

First of all I want to emphasize that this bug does not mean that no scaling of the IRB was implemented. The bug was just a silent fall back to the simple scaling also the user specified the 2-dimensional sampling.

As Arjen said I have made some tests, comparing the mean free path in the IRB field. Exemplary, the Kneiske 2004 field is attached. It is clearly visible that differences in the mean free path exist, comparing the two-dimensional interpolation (haveRedshiftDependence=True) and the simpler scaling of the field (haveRedshiftDependence=False) IRB_Kneiske04.pdf.

In addition, I have also checked the influence of the different redshift scaling on actual simualtion output. In doing so, I have slightly adjusted the example for neutrino production. I decreased the energy threshold to 1PeV and increased the statistics to 120,000 primaries. Furthermore, I deactivated all modules but the PhotoPionProduction (IRB_x). The difference between the two implementations becomes largest at the lowest energies
Kneiske04_neutrinoDif . Also this may seem quite large, the differences between different EBL models are at least on the same order of magnitude. For other models (e.g. Finke10) the disagreement between the two redshift treatments can be much smaller (~percent level).

Notes:

  • The differences between two-dimensional interpolation and scaling of the redshift have been one of the suggested reasons to explain differences between SimProp and CRPropa (see arXiv:1901.01244). This could be re-checked with the 2-dim sampling now available in CRPropa.

  • At the moment it is not suggested to use the 2-dim sampling together with other loss modules (PhotoDisintegration, etc.). Since the 2-dim interpolation is only available for PPP an inconsistent treatment of the EBL would otherwise be introduced. The latest commit introduces a KISS_LOG_WARNING to inform the user.

  • The 2-dim sampling should become the default treatment for all loss processes in the future.

Best,
Lukas

PS: If want to see plots also for other IRB implementation please write me an email.

@lukasmerten
Copy link
Member Author

Thanks for giving feedback on this issue - internally and here. I am merging.

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

Successfully merging this pull request may close these issues.

Redshift evolution in PhotoPionProduction
4 participants