StripEnergyThresholdFinder for per-strip slow and fast threshold extraction with diagnostics#166
Draft
JarredMRoberts wants to merge 4 commits into
Draft
Conversation
Author
|
I still need to fix all of the code style issues and work on some optimizations to speed the code up a bit. |
fhagemann
reviewed
Jun 23, 2026
Comment on lines
+77
to
+89
| struct StripKey | ||
| { | ||
| int det; | ||
| char side; | ||
| int strip; | ||
|
|
||
| bool operator<(const StripKey& o) const | ||
| { | ||
| if(det!=o.det) return det<o.det; | ||
| if(side!=o.side) return side<o.side; | ||
| return strip<o.strip; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Can you not reuse the code from MStripMap.cxx here?
Comment on lines
+97
to
+189
| class EnergyCalHelper | ||
| { | ||
| public: | ||
|
|
||
| struct Coeff | ||
| { | ||
| double c0=0; | ||
| double c1=0; | ||
| double c2=0; | ||
| double c3=0; | ||
| }; | ||
|
|
||
| bool Load(const string& fileName,int nDet=64,int nStrip=65) | ||
| { | ||
| m_NDet=nDet; | ||
| m_NStrip=nStrip; | ||
|
|
||
| m_Coeffs.resize(m_NDet); | ||
|
|
||
| for(int d=0;d<m_NDet;d++) | ||
| { | ||
| m_Coeffs[d].resize(2); | ||
|
|
||
| for(int s=0;s<2;s++) | ||
| m_Coeffs[d][s].resize(m_NStrip); | ||
| } | ||
|
|
||
| ifstream in(fileName); | ||
|
|
||
| if(!in) | ||
| { | ||
| cout<<"Unable to open calibration file "<<fileName<<endl; | ||
| return false; | ||
| } | ||
|
|
||
| string line; | ||
|
|
||
| while(getline(in,line)) | ||
| { | ||
| if(line.empty()) continue; | ||
|
|
||
| stringstream ss(line); | ||
|
|
||
| string tag; | ||
| ss>>tag; | ||
|
|
||
| if(tag!="CM") continue; | ||
|
|
||
| string unused; | ||
| int det=-1; | ||
| int strip=-1; | ||
| string side; | ||
| string order; | ||
|
|
||
| ss>>unused>>det>>strip>>side>>order; | ||
|
|
||
| if(det<0||det>=m_NDet) continue; | ||
| if(strip<0||strip>=m_NStrip) continue; | ||
|
|
||
| int sideInt=(side=="l")?0:1; | ||
|
|
||
| Coeff c; | ||
|
|
||
| if(order=="poly1zero") | ||
| ss>>c.c1; | ||
| else if(order=="poly1") | ||
| ss>>c.c0>>c.c1; | ||
| else if(order=="poly2") | ||
| ss>>c.c0>>c.c1>>c.c2; | ||
| else | ||
| ss>>c.c0>>c.c1>>c.c2>>c.c3; | ||
|
|
||
| m_Coeffs[det][sideInt][strip]=c; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| double ADCToEnergy(int det,char side,int strip,double adc) const | ||
| { | ||
| int sideInt=(side=='l')?0:1; | ||
| const Coeff& c=m_Coeffs[det][sideInt][strip]; | ||
|
|
||
| return c.c3*pow(adc,3)+c.c2*pow(adc,2)+c.c1*adc+c.c0; | ||
| } | ||
|
|
||
| private: | ||
|
|
||
| int m_NDet; | ||
| int m_NStrip; | ||
|
|
||
| vector<vector<vector<Coeff>>> m_Coeffs; | ||
| }; |
There was a problem hiding this comment.
This essentially does what MModuleEnergyCalibration does.
Can you create a MModuleEnergyCalibration variable in this app and use its functions here to avoid code duplication and keep the code for this app short?
Comment on lines
+195
to
+251
| class TACCalHelper | ||
| { | ||
| public: | ||
|
|
||
| struct Coeff | ||
| { | ||
| double slope = 0; | ||
| double offset = 0; | ||
| }; | ||
|
|
||
| bool Load(const string& fileName) | ||
| { | ||
| ifstream in(fileName); | ||
| if(!in) | ||
| { | ||
| cout<<"Failed to open TAC calibration file: "<<fileName<<endl; | ||
| return false; | ||
| } | ||
|
|
||
| string line; | ||
| getline(in,line); // skip header | ||
|
|
||
| while(getline(in,line)) | ||
| { | ||
| if(line.empty()) continue; | ||
|
|
||
| stringstream ss(line); | ||
|
|
||
| int strip_id, det, side, strip; | ||
| double slope, slope_err, offset, offset_err; | ||
|
|
||
| ss >> strip_id >> det >> side >> strip | ||
| >> slope >> slope_err >> offset >> offset_err; | ||
|
|
||
| char sideChar = (side==0) ? 'l' : 'h'; | ||
|
|
||
| StripKey key{det, sideChar, strip}; | ||
|
|
||
| m_Coeffs[key] = {slope, offset}; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| double TACToEnergy(int det, char side, int strip, double tac) const | ||
| { | ||
| StripKey key{det, side, strip}; | ||
|
|
||
| auto it = m_Coeffs.find(key); | ||
| if(it == m_Coeffs.end()) return 0; | ||
|
|
||
| return it->second.slope * tac + it->second.offset; | ||
| } | ||
|
|
||
| private: | ||
| map<StripKey, Coeff> m_Coeffs; | ||
| }; |
There was a problem hiding this comment.
This already exists in MModuleTACcut --> use existing class to avoid code duplication here.
| private: | ||
|
|
||
| // --- Configuration --- | ||
| EnergyCalHelper m_EnergyCal; |
There was a problem hiding this comment.
Suggested change
| EnergyCalHelper m_EnergyCal; | |
| MModuleEnergyCalibration m_EnergyCal; |
| #include "MReadOutAssembly.h" | ||
| #include "MStripHit.h" | ||
| #include "MString.h" | ||
|
|
There was a problem hiding this comment.
Suggested change
| #include "MModuleEnergyCalibration.h" | |
| #include "MModuleTACcut.h" | |
| #include "MStripMap.h" |
|
|
||
|
|
||
| //EnergyCalHelper m_EnergyCal; | ||
| TACCalHelper m_EnergyCal_TAC; |
There was a problem hiding this comment.
That's a weird name, is Energy intended in this name? I would just call it m_TACCal.. And if it's called m_..., consider making this a (private) member variable of the class -- just like m_EnergyCal
Also: where is this TACCalHelper ever used?
| //EnergyCalHelper m_EnergyCal; | ||
| TACCalHelper m_EnergyCal_TAC; | ||
|
|
||
| if(!m_EnergyCal.Load(m_CalibrationFile.Data())) |
There was a problem hiding this comment.
MModuleEnergyCalibration has a ReadEnergyCalibrationFile function that you could call here instead:
Suggested change
| if(!m_EnergyCal.Load(m_CalibrationFile.Data())) | |
| if (m_EnergyCal.ReadEnergyCalibrationFile(m_CalibrationFile) == false) |
| EnergyCalHelper m_EnergyCal; | ||
| vector<string> m_InputFiles; | ||
| MString m_CalibrationFile; | ||
| MString m_TACCalibrationFile; |
fhagemann
requested changes
Jun 23, 2026
Co-authored-by: Felix Hagemann <hagemann@berkeley.edu>
Co-authored-by: Felix Hagemann <hagemann@berkeley.edu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a standalone application, StripEnergyThresholdFinder, for computing per-strip slow and fast energy thresholds. Slow thresholds are determined from ADC spectra using a noise peak and trough method, while fast thresholds are determined from dt0/dt1 timing crossover. The tool reads calibrated HDF5 data via MModuleLoaderMeasurementsHDF, applies strip mapping and energy calibration from a YAML configuration, and produces ROOT diagnostic outputs (energy spectra with thresholds, dt0 vs dt1 per strip, and threshold distributions) along with CSV export files. The implementation is self-contained under apps/ and does not modify existing modules. Tested on COSI datasets with consistent threshold behavior and expected diagnostic results. Target branch is develop/em.