Repository navigation
Feature/aantonak caf truthfix - #674
aantonakis wants to merge 8 commits into
Conversation
…d by CAFMaker's FillTrueG4Particle function. Debug statements are currently included.
…s wtih missing parents
…sed parent IDs. The IDs were not guaranteed to be uniquie when multiple MCTruths are present.
tjones79
left a comment
tjones79
left a comment
There was a problem hiding this comment.
Thanks @aantonakis, this looks great!
There are a few comments as I was trying to figure out Genie -> G4 indices. I hope they are helpful
|
|
||
| // Now we can check if we need to fill this missed parent in the CAF | ||
| if (matchedGenie->Mother() != min_g4_track_id-1) { | ||
| const simb::MCParticle& missedParent = inventoryTruth->GetParticle(matchedGenie->Mother()); |
There was a problem hiding this comment.
Is it guaranteed that the GENIE TrackId always perfectly matches its index in the MCTruth vector? (Also asking to gain knowledge for myself!) For instance, if intermediate particles get filtered out or IDs skip numbers? Passing Mother() directly into GetParticle() in that case could access the wrong particle or cause an out-of-bounds
I guess a more robust way to do this would be to safely look up the parent by explicitly searching for the matching TrackId
| if (matchedGenie) { | ||
|
|
||
| // Now we can check if we need to fill this missed parent in the CAF | ||
| if (matchedGenie->Mother() != min_g4_track_id-1) { |
There was a problem hiding this comment.
I had thought that GENIE's 'no mother' value is typically -1, not the G4 boundary (min_g4_track_id-1, which is usually 0, as I believe you have used correctly above).
If that is the case, matchedGenie is a GENIE particle, so if it has no mother, it will pass the != 0 check and cause an out-of-bounds lookup at index -1. So should this be changed?
| caf::SRTrueParticle &srparticle, int interaction_id, int id_offset) { | ||
|
|
||
| std::vector<std::pair<geo::WireID, const sim::IDE *>> empty; | ||
| const std::vector<std::pair<geo::WireID, const sim::IDE *>> &particle_ides = id_to_ide_map.count(particle.TrackId()) ? id_to_ide_map.at(particle.TrackId()) : empty; |
There was a problem hiding this comment.
Similar to my comment/question on the module file
Are the keys in id_to_ide_map and id_to_truehit_map Geant4 track IDs?
Since particle here is a GENIE particle, I'm slightly concerned that particle.TrackId() might accidentally collide with an unrelated Geant4 track ID, giving this unpropagated particle someone else's detector hits. Since these GENIE particles didn't go through Geant4 anyway, can we safely just remove these map lookups and leave their visE and nhit at 0?
|
|
||
| // Special GENIE particles get a shifted positive ID so they do not overlap with G4 track IDs. | ||
| srparticle.G4ID = particle.TrackId() + id_offset; | ||
| srparticle.parent = particle.Mother() > 0 ? static_cast<unsigned>(particle.Mother() + id_offset) : 0u; |
There was a problem hiding this comment.
Again, similar to previous comments
I think GENIE uses -1 for particles with no mother, whereas Geant4 uses 0. Please disregard this and other comments if I'm incorrect!
In that case, if a GENIE particle's mother has a track ID of 0 (neutrino), > 0 will evaluate to false and return 0u instead of 0 + id_offset, right? Should this be particle.Mother() != -1 instead?
|
|
||
| // Save the daughter particles | ||
| for (int i_d = 0; i_d < particle.NumberDaughters(); i_d++) { | ||
| srparticle.daughters.push_back(particle.Daughter(i_d)); |
There was a problem hiding this comment.
Does this need to be particle.Daughter(i_d) + id_offset?
| // Incoming neutrino. | ||
|
|
||
| if (particle.TrackId() == | ||
| truth.GetNeutrino().Nu().TrackId()) |
There was a problem hiding this comment.
Will this work if the module runs over a file that includes Cosmic Ray Overlays or Particle Gun events? (Cases where MCTruth record contains no neutrino)
3 participants
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.
This PR adds missing truth information to the CAFMaker output. Some unstable particles produced in the neutrino interaction such as eta mesons are missing in the CAF truth outputs because they are not propagated to Geant4. The goal of this PR is to identify primary particles of the interaction which are missing their parent particles and to loop back through the decay chain in the generator truth to add in the missing information. This is described in more detail in SBN docdb entry 49027.