Removed HasTriggered variable from MStripHit - #202
Conversation
There was a problem hiding this comment.
I made adjustments to this PR to get the unit level tests to pass again.
However, we might want to think, that instead of completely removing the information on m_HasTriggered to an output file, we might want to replace it with the information in m_IsNearestNeighbor instead.
d5926e9 to
4a6ca8e
Compare
fhagemann
left a comment
fhagemann
left a comment
There was a problem hiding this comment.
I force-pushed making this PR non-breaking when it comes to reading/writing to DAT/ROA files (by replacing m_HasTriggered streaming with (the inverse of) m_IsNearestNeighbor.
| <<(m_IsNearestNeighbor == false)<<" " | ||
| <<setprecision(9)<<m_Timing<<" " | ||
| <<m_ADCUnits<<" " | ||
| <<m_Energy<<" " | ||
| <<m_EnergyResolution<<" " | ||
| <<MakeFlags()<<endl; |
There was a problem hiding this comment.
Ok, I now replaced all instances of m_HasTriggered when writing/reading ROA/DAT files to (the inverse of) m_IsNearestNeighbor.
Only thing is: it looks like we're writing redundant data write now (and also did before), as the information if a strip hit was a "nearest neighbor" is saved both through the previous m_HasTriggered slot and the strip flags via MakeFlags().
There was a problem hiding this comment.
You're right about the MakeFlags(). Andreas is planning an overhaul on the .dat reading/writing (PR #179 ), so I'm okay will leaving this redundancy for now. I'm not sure how many people are using the .dat format, but the bitmask for MakeFlags takes a minute to get used to...
|
I moved the commit to add |
|
Thanks for handling the UnitTests! I had completely forgotten to check those files 😬 Things look good, so I'll merge now. |
2 participants
As agreed upon in the August 18th CDEE meeting, we're removing the HasTriggered variable from MStripHit. This variable wasn't used in the HDF measurement loader, and it is redundant (but opposite to) IsNearestNeighbor.
Note that I kept the HasTriggered variable as part of the MShieldCrystalHit and MCrystalHit class (it seems like one of these is a duplicate...)