StripEnergyThresholdFinder for per-strip slow and fast threshold extraction with diagnostics - #166
JarredMRoberts wants to merge 36 commits into
Conversation
|
I still need to fix all of the code style issues and work on some optimizations to speed the code up a bit. |
Co-authored-by: Felix Hagemann <hagemann@berkeley.edu>
Co-authored-by: Felix Hagemann <hagemann@berkeley.edu>
…nventions and integrate energy calibration improvements
There was a problem hiding this comment.
Hi Jarred,
I had an extensive look at this app and finally got this to run.
I replied to all comments in this PR, and opened a separate PR onto your fork/branch, addressing some code changes to
- get this app running
- remove the helper classes by replacing them with existing nuclearizer/megalib code
Here is the PR onto your branch with detailed code changes: JarredMRoberts#1
|
I second not to have an additional parser linked. |
fhagemann
left a comment
There was a problem hiding this comment.
One more remark: I got this app to run and got reasonable results for the fast and slow thresholds only when using a dataset taken with NN off.
We might want to always filter out NN events, for this app to also run using datasets taken with NN on.
…ise peak range from ADC to keV
fhagemann
left a comment
There was a problem hiding this comment.
This is a quick review with code-style comments, I will follow up with some more general comments
fhagemann
left a comment
There was a problem hiding this comment.
This is a quick review with code-style comments, I will follow up with some more general comments
Co-authored-by: Felix Hagemann <hagemann@berkeley.edu>
fhagemann
left a comment
There was a problem hiding this comment.
Some general comments:
- do you have some figures to walk us through the pedestal/peak finding algorithms, to get an idea in what cases the pedestal, flattop or peak detection loops would do their thing?
- we might want to avoid having hard-coded values distributed throughout the file and define them as constants at the top of the file (or member variables of the threshold app class) instead.
- In the end, we might want to generate ONE threshold file per threshold type (fast, slow, hardware), instead of splitting them into LV and HV subfiles. I believe that the forward pipeline will require a combined file in the end anyhow
| // --- dt0 vs dt1 separation --- | ||
| bool is_dt1 = (TAC > 8000); // initial threshold |
There was a problem hiding this comment.
Is this ever not the case?
Also, should this hard-coded value become some member variable or be defined at the very beginning of the file, instead of very far into the code?
There was a problem hiding this comment.
Looking at this in more detail: shouldn't this be:
| // --- dt0 vs dt1 separation --- | |
| bool is_dt1 = (TAC > 8000); // initial threshold | |
| // --- dt0 vs dt1 separation --- | |
| bool is_dt1 = SH->HasFastTiming(); |
??
There was a problem hiding this comment.
Tested on data from HP52406-1 (Am-241 lifetime data, taken with HDFv2.2: gse_20260217T115220.hdf5):
With the current TAC > 8000 line, I get nothing in HV strip 32 (no dt1 distribution, everything seems to be classified as dt0), resulting in the fallback fast threshold:
WARNING: FAST threshold could not be determined for Det 0 Side HV Strip 32: insufficient timing populations (dt0=88441, dt1=293, dt1 fraction=0.3302%). Using fallback threshold = 34 keV.
Changing it to SH->HasFastTiming() results in reasonable dt0 and dt1 distributions, but the fast threshold seems to be determined incorrectly:

This is my XML config file (using strip map and ecal file from the unit tests):
<?xml version="1.0" encoding="UTF-8"?>
<StripEnergyThresholdFinder>
<Input>
<DataFiles>
<DataFile>/home/hagemann/Downloads/gse_20260217T115220.hdf5</DataFile>
</DataFiles>
<CalibrationFile>/home/hagemann/Software/COSItools/nuclearizer/resource/unittestdata/406-1/hp52406-1.full.ecal</CalibrationFile>
<StripMap>/home/hagemann/Software/COSItools/nuclearizer/resource/unittestdata/406-1/hp52406-1.stripmap.map</StripMap>
</Input>
<Output>
<Prefix>output_file_prefix</Prefix>
</Output>
<Analysis>
<MinEntries>10</MinEntries>
<FallbackThresholdKeV>20</FallbackThresholdKeV>
<NoiseSearchMaxKeV>40</NoiseSearchMaxKeV>
<FastFallbackKeV>34</FastFallbackKeV>
</Analysis>
</StripEnergyThresholdFinder>There was a problem hiding this comment.
Whatever was broken here is fixed now. I've never been able to reproduce this behavior.
There was a problem hiding this comment.
Unresolving this, because I don't think this is true.
When using your current is_dt1 classification, you sometimes get ONLY dt0 events (see your Notes for HV strip 32 for all detectors tested with the faulty NICEv1 board).
I get reasonable dt0 and dt1 distributions ONLY if I incorporate the change listed above:
// --- dt0 vs dt1 separation ---
bool is_dt1 = SH->HasFastTiming();
fhagemann
left a comment
There was a problem hiding this comment.
I can confirm that the app compiles and works when using multiple files.
You might want to remove the index from the XML file (it also works without it).
… Hardware). Added hardware diagnostics in ADC units
…ead strips) gracefully. Added warning to failed fits in hardware thresholds
|
I've put together some slides that go into some detail about the app, what the inputs and outputs are, and how the various algorithms work. (https://docs.google.com/presentation/d/1_sveTiA17fUNNXd2h1dcrlFbWuK2XNRssNNchlbn5ZY/edit?usp=sharing) |
…nd updated XML file format for multiple file reading
|
Thanks for creating hardware threshold files and uploading them here: https://drive.google.com/drive/folders/1FN4fp0_imv-opuJHoKn1N3DPwUUPe38A?usp=sharing I opened PR #208 based on those, but had to make some minor adjustments to be consistent with the format of
There have also been some changes to the TAC calibration module in PR #194, and I will take care of resolving the merge conflicts here. |
| // --- dt0 vs dt1 separation --- | ||
| bool is_dt1 = (TAC > FAST_TIMING_TAC_THRESHOLD); // initial threshold |
There was a problem hiding this comment.
I still believe this is wrong and should be
| // --- dt0 vs dt1 separation --- | |
| bool is_dt1 = (TAC > FAST_TIMING_TAC_THRESHOLD); // initial threshold | |
| // --- dt0 vs dt1 separation --- | |
| bool is_dt1 = SH->HasFastTiming(); |
There was a problem hiding this comment.
In your notes, you show these spectra for some detectors with HV strip 32 (the ones tested with the faulty NICEv1 board), which I can reproduced with the current is_dt1 = (TAC > FAST_TIMING_TAC_THRESHOLD);:

When I update the line to bool is_dt1 = SH->FastTiming();, this is what I get:

So, using bool is_dt1 = SH->FastTiming(); seems to give a more reasonable classification into dt0 and dt1 events, but the fit result for the fast threshold seems to be off.
There was a problem hiding this comment.
I plotted this histogram from the same datafile using Julia, and also plotted the fast threshold that's determined with this app (1901 ADC units) in ADC space, and this is consistent with what we are getting in this app:

Something causes the fast threshold to be wrong here, I will try to find out what it is.
There was a problem hiding this comment.
Ok, I found the reason:
The fast threshold finding algorithm finds the first bin where there are more dt1 events (with fast timing) than dt0 events (with slow timing), but also requires there to be at least 50 events in that bin in general.
(constexpr int FAST_MIN_CROSSOVER_COUNTS = 50;).
What happens here, is that this default of 50 is too low, so it sets the threshold at the first bin where the total number exceeded 50:

We might want to add some check that look backwards if the previous bin (regardless if it had less than 50 counts) had more dt0 than dt1 events, or shift the fast threshold value back down if that isn't the case.
There was a problem hiding this comment.
This seems to work (at least in Julia, would need to translate this into C++):
using StatsBase
dt1 = fit(Histogram, is_dt1.energy, 1600:1:2100)
dt0 = fit(Histogram, is_dt0.energy, 1600:1:2100)
for i in eachindex(dt0.weights)
# Initially ignore all bins that have fewer than 50 counts
if dt0.weights[i] + dt1.weights[i] < 50
continue
end
# Find the first bin with more than 50 counts with more dt1 than dt0 events
if dt0.weights[i] < dt1.weights[i]
println("Initial guess: ", first(dt0.edges)[i])
# If that was also the case for previous bins: reduce i
while dt0.weights[i-1] < dt1.weights[i-1]
i -= 1
end
println("Backward search: ", first(dt0.edges)[i])
break
end
endInitial guess: 1901
Backward search: 1793
bd7161b to
20de738
Compare
| if (n1 > n0) { | ||
| crossoverADC = ADC_val; | ||
| break; | ||
| } |
There was a problem hiding this comment.
| if (n1 > n0) { | |
| crossoverADC = ADC_val; | |
| break; | |
| } | |
| if (n1 > n0) { | |
| crossoverADC = ADC_val; | |
| // Make sure that this is an actual crossover and not just the first bin | |
| // with more than FAST_MIN_CROSSOVER_COUNTS where n1 was above n0 | |
| while (n1 > n0 && ADC_val >= first_nonzero) { | |
| crossoverADC = ADC_val; | |
| ADC_val--; | |
| if (ADCMap.find(ADC_val) != ADCMap.end()) { | |
| n0 = ADCMap[ADC_val].first; | |
| n1 = ADCMap[ADC_val].second; | |
| } | |
| } |
Adding this backward search seems to resolve the issue above:

There was a problem hiding this comment.
All major changes I requested are implemented.



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.