Skip to content

Allow for different thresholds for OpHitFinder #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
asanchezcastillo wants to merge 7 commits into
base: develop
Choose a base branch
from
Draft

Allow for different thresholds for OpHitFinder #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d09f49d
Change type pmtana::Waveform_t to raw::OpDetWaveform for OpHitFinder …
asanchezcastillo Jan 17, 2025
885fe73
larana v09_15_15 for larsoft v09_93_01
lgarren Oct 29, 2024
4a8cca3
Add individual channel threshold for ophitfinding algorithms
asanchezcastillo Jan 17, 2025
3d534da
Merge pull request #1 from LArSoft/develop
asanchezcastillo Jan 21, 2025
9d845cc
Format code
asanchezcastillo Jan 27, 2025
a1f9638
Merge remote-tracking branch 'origin' into develop
asanchezcastillo Feb 5, 2025
af6e19c
Merge pull request #2 from LArSoft/develop
asanchezcastillo Feb 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ cmake_minimum_required(VERSION 3.20 FATAL_ERROR)
find_package(cetmodules 3.12.00 REQUIRED)
project(larana VERSION 10.00.09 LANGUAGES CXX)


# cetbuildtools contains our cmake modules

include(CetCMakeEnv)
Expand Down
12 changes: 11 additions & 1 deletion larana/OpticalDetector/OpHitFinder/AlgoCFD.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ namespace pmtana {
_D = pset.get<int>("Delay");

//_number_presample = pset.get<int> ("BaselinePreSample");
_peak_thresh_by_channel = pset.get<bool>("ThreshByChannel", false);
_peak_thresh_vector = pset.get<std::vector<double>>("PeakThreshVector", {});
Comment on lines +34 to +35
Copy link

@absolution1 absolution1 Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this module is not using fcl validation, so default fcl values should be avoided. Could these be removed from the producer, and then add the parameters to the default fcl?

Copy link
Member

@knoepfel knoepfel Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We suggest a different approach. It looks like two configuration parameters are being used: one that says "use the threshold-by-channel option", and another that says "here are the thresholds by channel". Instead of using two parameters, it is better to use one optional parameter. For example, make this change:

-    //Whether to apply an individual threshold for each channel
-    bool _peak_thresh_by_channel;
-    //Vector of thresholds for each channel
-    std::vector<double> _peak_thresh_vector;
+    //Optional user-specified vector of thresholds for each channel
+    std::optional<std::vector<double>> _peak_thresh_vector;

You would then switch to:

  _peak_thresh_vector = pset.get_if_present<std::vector<double>>("PeakThreshVector");

If _peak_thresh_vector is "engaged" (i.e. there's a value associated with it), then "PeakThreshVector" has been specified in the configuration. If it is not engaged, then there is no configuration parameter present. The check would then look something like:

    if (_peak_thresh_vector) {
      uint ChannelNumber = wf.ChannelNumber();
      if (ChannelNumber >= _peak_thresh_vector->size())
        throw cet::exception("OpHitFinder")
          << "Threshold not found for channel " << ChannelNumber << "\n";
      _peak_thresh = (*_peak_thresh_vector)[ChannelNumber];
    }

_peak_thresh = pset.get<double>("PeakThresh");
_start_thresh = pset.get<double>("StartThresh");
_end_thresh = pset.get<double>("EndThresh");
Expand All @@ -48,14 +50,22 @@ namespace pmtana {
}

//***************************************************************
bool AlgoCFD::RecoPulse(const pmtana::Waveform_t& wf,
bool AlgoCFD::RecoPulse(const raw::OpDetWaveform& wf,
const pmtana::PedestalMean_t& mean_v,
const pmtana::PedestalSigma_t& sigma_v)
//***************************************************************
{

Reset();

if (_peak_thresh_by_channel) {
uint ChannelNumber = wf.ChannelNumber();
if (ChannelNumber >= _peak_thresh_vector.size())
throw cet::exception("OpHitFinder")
<< "Threshold not found for channel " << ChannelNumber << "\n";
_peak_thresh = _peak_thresh_vector[ChannelNumber];
}

std::vector<double> cfd;
cfd.reserve(wf.size());

Expand Down
6 changes: 5 additions & 1 deletion larana/OpticalDetector/OpHitFinder/AlgoCFD.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ namespace pmtana {

protected:
/// Implementation of AlgoCFD::reco() method
bool RecoPulse(const pmtana::Waveform_t&,
bool RecoPulse(const raw::OpDetWaveform&,
const pmtana::PedestalMean_t&,
const pmtana::PedestalSigma_t&);

Expand All @@ -64,6 +64,10 @@ namespace pmtana {
double _peak_thresh;
double _start_thresh;
double _end_thresh;
//Whether to apply an individual threshold for each channel
bool _peak_thresh_by_channel;
//Vector of thresholds for each channel
std::vector<double> _peak_thresh_vector;
};

}
Expand Down
2 changes: 1 addition & 1 deletion larana/OpticalDetector/OpHitFinder/AlgoFixedWindow.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ namespace pmtana {
}

//***************************************************************
bool AlgoFixedWindow::RecoPulse(const Waveform_t& wf,
bool AlgoFixedWindow::RecoPulse(const raw::OpDetWaveform& wf,
const PedestalMean_t& mean_v,
const PedestalSigma_t& sigma_v)
//***************************************************************
Expand Down
2 changes: 1 addition & 1 deletion larana/OpticalDetector/OpHitFinder/AlgoFixedWindow.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ namespace pmtana {

protected:
/// Implementation of AlgoFixedWindow::reco() method
bool RecoPulse(const pmtana::Waveform_t&,
bool RecoPulse(const raw::OpDetWaveform&,
const pmtana::PedestalMean_t&,
const pmtana::PedestalSigma_t&);

Expand Down
13 changes: 11 additions & 2 deletions larana/OpticalDetector/OpHitFinder/AlgoSiPM.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ namespace pmtana {
const std::string name)
: PMTPulseRecoBase(name)
{

_adc_thres_by_channel = pset.get<bool>("ADCThresholdByChannel", false);
Copy link

@absolution1 absolution1 Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment about default values

_adc_thres_vector = pset.get<std::vector<float>>("ADCThresVector", {});
Copy link

@absolution1 absolution1 Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment about default values

_adc_thres = pset.get<float>("ADCThreshold");
_min_width = pset.get<float>("MinWidth");
_2nd_thres = pset.get<float>("SecondThreshold");
Expand All @@ -34,7 +35,7 @@ namespace pmtana {
}

//---------------------------------------------------------------------------
bool AlgoSiPM::RecoPulse(const pmtana::Waveform_t& wf,
bool AlgoSiPM::RecoPulse(const raw::OpDetWaveform& wf,
const pmtana::PedestalMean_t& ped_mean,
const pmtana::PedestalSigma_t& ped_rms)
{
Expand All @@ -49,6 +50,14 @@ namespace pmtana {
ped_mean
.front(); //Switch pedestal definition to incoroprate pedestal finder - K.S. 04/18/2019

if (_adc_thres_by_channel) {
uint ChannelNumber = wf.ChannelNumber();
if (ChannelNumber >= _adc_thres_vector.size())
throw cet::exception("OpHitFinder")
<< "ADC Threshold not found for channel " << ChannelNumber << "\n";
_adc_thres = _adc_thres_vector[ChannelNumber];
}

double threshold = _adc_thres;
threshold += pedestal;
double pre_threshold = _2nd_thres;
Expand Down
8 changes: 7 additions & 1 deletion larana/OpticalDetector/OpHitFinder/AlgoSiPM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,13 @@ namespace pmtana {
// void SetNSigma(double v) {_nsigma = v;};

protected:
bool RecoPulse(const pmtana::Waveform_t&,
bool RecoPulse(const raw::OpDetWaveform&,
const pmtana::PedestalMean_t&,
const pmtana::PedestalSigma_t&);

//Whether to apply an individual threshold for each channel
bool _adc_thres_by_channel;

// A variable holder for a user-defined absolute ADC threshold value
double _adc_thres;

Expand All @@ -55,6 +58,9 @@ namespace pmtana {
// Use this pedestal instead of the one given by the pedestal algorithm
double _pedestal;

// Vector of ADCThreshold to be used for each channel
std::vector<float> _adc_thres_vector;

// A variable holder for a multiplicative factor for the pedestal
// standard deviation to define the threshold
// double _nsigma;
Expand Down
19 changes: 17 additions & 2 deletions larana/OpticalDetector/OpHitFinder/AlgoSlidingWindow.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ namespace pmtana {
{
_positive = pset.get<bool>("PositivePolarity", true);

_adc_thres_by_channel = pset.get<bool>("ADCThresholdByChannel", false);

_adc_thres_vector = pset.get<std::vector<float>>("ADCThresVector", {});

Comment on lines +29 to +32
Copy link

@absolution1 absolution1 Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default values

_adc_thres = pset.get<float>("ADCThreshold");

_tail_adc_thres = pset.get<float>("TailADCThreshold", _adc_thres);
Expand Down Expand Up @@ -59,7 +63,7 @@ namespace pmtana {
}

//***************************************************************
bool AlgoSlidingWindow::RecoPulse(const pmtana::Waveform_t& wf,
bool AlgoSlidingWindow::RecoPulse(const raw::OpDetWaveform& wf,
const pmtana::PedestalMean_t& mean_v,
const pmtana::PedestalSigma_t& sigma_v)
//***************************************************************
Expand Down Expand Up @@ -87,6 +91,17 @@ namespace pmtana {

Reset();

if (_adc_thres_by_channel) {
uint ChannelNumber = wf.ChannelNumber();
if (ChannelNumber >= _adc_thres_vector.size())
throw cet::exception("OpHitFinder")
<< "ADC Threshold not found for channel " << ChannelNumber << "\n";
_adc_thres = _adc_thres_vector[ChannelNumber];
}

std::cout << " The channel number is " << wf.ChannelNumber() << " and the adc threshold is "
<< _adc_thres << std::endl;

for (size_t i = 0; i < wf.size(); ++i) {

double value = 0.;
Expand Down Expand Up @@ -298,4 +313,4 @@ namespace pmtana {
return true;
}

}
}
11 changes: 9 additions & 2 deletions larana/OpticalDetector/OpHitFinder/AlgoSlidingWindow.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ namespace fhicl {
}

#include "larana/OpticalDetector/OpHitFinder/OpticalRecoTypes.h"
#include "lardataobj/RawData/OpDetWaveform.h"

#include <string>

Expand All @@ -47,13 +48,16 @@ namespace pmtana {

protected:
/// Implementation of AlgoSlidingWindow::reco() method
bool RecoPulse(const pmtana::Waveform_t&,
bool RecoPulse(const raw::OpDetWaveform&,
const pmtana::PedestalMean_t&,
const pmtana::PedestalSigma_t&);

/// A boolean to set waveform positive/negative polarity
bool _positive;

//Whether to apply an individual threshold for each channel
bool _adc_thres_by_channel;

/// A variable holder for a user-defined absolute ADC threshold value
float _adc_thres, _tail_adc_thres, _end_adc_thres;

Expand All @@ -64,9 +68,12 @@ namespace pmtana {
float _nsigma, _tail_nsigma, _end_nsigma;
bool _verbose;
size_t _num_presample, _num_postsample;

// Vector of ADCThreshold to be used for each channel
std::vector<float> _adc_thres_vector;
};

}
#endif

/** @} */ // end of doxygen group
/** @} */ // end of doxygen group
2 changes: 1 addition & 1 deletion larana/OpticalDetector/OpHitFinder/AlgoThreshold.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ namespace pmtana {
}

//***************************************************************
bool AlgoThreshold::RecoPulse(const Waveform_t& wf,
bool AlgoThreshold::RecoPulse(const raw::OpDetWaveform& wf,
const PedestalMean_t& mean_v,
const PedestalSigma_t& sigma_v)
//***************************************************************
Expand Down
2 changes: 1 addition & 1 deletion larana/OpticalDetector/OpHitFinder/AlgoThreshold.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ namespace pmtana {

protected:
/// Implementation of AlgoThreshold::reco() method
bool RecoPulse(const pmtana::Waveform_t& wf,
bool RecoPulse(const raw::OpDetWaveform& wf,
const pmtana::PedestalMean_t& mean_v,
const pmtana::PedestalSigma_t& sigma_v);

Expand Down
2 changes: 1 addition & 1 deletion larana/OpticalDetector/OpHitFinder/PMTPulseRecoBase.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace pmtana {
}

//******************************************************************
bool PMTPulseRecoBase::Reconstruct(const Waveform_t& wf,
bool PMTPulseRecoBase::Reconstruct(const raw::OpDetWaveform& wf,
const PedestalMean_t& mean_v,
const PedestalSigma_t& sigma_v)
//******************************************************************
Expand Down
5 changes: 3 additions & 2 deletions larana/OpticalDetector/OpHitFinder/PMTPulseRecoBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include "OpticalRecoTypes.h"
#include "larana/OpticalDetector/OpHitFinder/RiseTimeTools/RiseTimeCalculatorBase.h"
#include "lardataobj/RawData/OpDetWaveform.h"

#include <memory>

Expand Down Expand Up @@ -84,7 +85,7 @@ namespace pmtana {
/** A core method: this executes the algorithm and stores reconstructed parameters
in the pulse_param struct object.
*/
bool Reconstruct(const pmtana::Waveform_t&,
bool Reconstruct(const raw::OpDetWaveform&,
const pmtana::PedestalMean_t&,
const pmtana::PedestalSigma_t&);

Expand All @@ -108,7 +109,7 @@ namespace pmtana {
bool _status;

protected:
virtual bool RecoPulse(const pmtana::Waveform_t&,
virtual bool RecoPulse(const raw::OpDetWaveform&,
const pmtana::PedestalMean_t&,
const pmtana::PedestalSigma_t&) = 0;

Expand Down
2 changes: 1 addition & 1 deletion larana/OpticalDetector/OpHitFinder/PulseRecoManager.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace pmtana {
}

//**********************************************************************
bool PulseRecoManager::Reconstruct(const pmtana::Waveform_t& wf) const
bool PulseRecoManager::Reconstruct(const raw::OpDetWaveform& wf) const
//**********************************************************************
{
if (_reco_algo_v.empty() && !_ped_algo)
Expand Down
3 changes: 2 additions & 1 deletion larana/OpticalDetector/OpHitFinder/PulseRecoManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#define PULSERECOMANAGER_H

#include "larana/OpticalDetector/OpHitFinder/OpticalRecoTypes.h"
#include "lardataobj/RawData/OpDetWaveform.h"

#include <vector>

Expand All @@ -37,7 +38,7 @@ namespace pmtana {
PulseRecoManager();

/// Implementation of ana_base::analyze method
bool Reconstruct(const pmtana::Waveform_t&) const;
bool Reconstruct(const raw::OpDetWaveform&) const;

/// A method to set pulse reconstruction algorithm
void AddRecoAlgo(pmtana::PMTPulseRecoBase* algo, PMTPedestalBase* ped_algo = nullptr);
Expand Down