Bugfix for FluxWeight and MeVPrtl #621
Conversation
… fix flux weight calculator to pick neutrino energy from MCTruth if MCFlux energy is -1
kjplows
moved this from Open pull requests
to Partially reviewed
in SBN software development
jicrespo
left a comment
jicrespo
left a comment
There was a problem hiding this comment.
Please see the comments below.
| } | ||
|
|
||
| flux.fFluxType = simb::kDk2Nu; | ||
| flux.fFluxType = simb::kSimple_Flux; |
There was a problem hiding this comment.
Why the flux type is "kSimple_Flux"? According to the MCFlux documentation, that item corresponds to "A simplified flux ntuple for quick running." It seems "kNtuple" (Full flux simulation ntuple) is more appropriate.
There was a problem hiding this comment.
I checked with Marco for which flux type to use and he suggested kSimple_Flux. Although it doesn't really matter which one of the two it is. Happy to switch to kNtuple.
| // Collect neutrino energy; mclist is replaced with fluxlist. | ||
| // double enu= mclist[inu].GetNeutrino().Nu().E(); | ||
| double enu= fluxlist[inu].fnenergyn; | ||
| if ( enu == -1 ) enu= mclist[inu].GetNeutrino().Nu().E(); //For MeVPrtl generator, not all MCFlux result in a neutrino, so equivalent neutrino energy is computed and saved as MCTruth |
There was a problem hiding this comment.
If MCFlux does not contain a neutrino, it seems unnecessary to add a fake neutrino energy different from -1. I think it is safer to keep neutrino energy = -1 and handle the exception downstream.
There was a problem hiding this comment.
The flux SBND weight module needs the neutrino energy to compute the systematic, see line: https://github.com/SBNSoftware/sbncode/blob/develop/sbncode/SBNEventWeight/Calculators/BNBFlux/FluxCalcPrep.cxx#L276
The equivalent neutrino energy is computed by MeVPrtl here:
sbncode/sbncode/EventGenerator/MeVPrtl/Tools/IMeVPrtlFlux.h
Lines 127 to 136 in 8b07405
There was a problem hiding this comment.
At the time being, the flux module would seg fault when the neutrino energy is missing or -1. So this fix is neccessary for at least the flux module to work with MeVPrtl inputs.
See @grayputnam 's comments from a while ago: https://github.com/SBNSoftware/sbncode/blob/develop/sbncode/EventGenerator/MeVPrtl/MeVPrtlGen_module.cc#L363-L371
|
|
||
| //or do the above 3 lines in one line | ||
| // auto const& mclist = *e.getValidHandle<std::vector<simb::MCTruth>>(fGeneratorModuleLabel); | ||
| auto const& mclist = *e.getValidHandle<std::vector<simb::MCTruth>>(fGeneratorModuleLabel); |
There was a problem hiding this comment.
If this line is only uncommented for the neutrino energy fix when MCFlux does not have a neutrino, I would move it to a block under if ( enu == -1 ). However, see my other comment about not fixing the energy in that case.
There was a problem hiding this comment.
There are several lines after this where mclist was commented out. So I'd like to keep at the same line for bookkeeping sake.
In MeVPrtl, by default, MCFlux neutrino energy is filled as -1 since not every meson in flux file would result in a neutrino. It instead computes an equivalent neutrino energy and saves it as MCTruth.
This change should not impact the neutrino workflow since every meson results in a neutrino, and MCFlux flux.nenergyn should always be filled.
Description
Please provide a detailed description of the changes this pull request introduces. If available, also link to a docdb link where the issue/change have been presented on/discussed.