Skip to content
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.

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

New waveform analysis class to interpolate waveforms using tapered sinc kernel #208

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/cmd/src/ProcBlockManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <RAT/TrueDAQProc.hh>
#include <RAT/WaveformAnalysis.hh>
#include <RAT/WaveformAnalysisLognormal.hh>
#include <RAT/WaveformAnalysisSinc.hh>
#include <RAT/WaveformPrep.hh>

namespace RAT {
Expand Down Expand Up @@ -94,6 +95,7 @@ ProcBlockManager::ProcBlockManager(ProcBlock *theMainBlock) {
AppendProcessor<WaveformAnalysis>();
AppendProcessor<WaveformPrep>();
AppendProcessor<WaveformAnalysisLognormal>();
AppendProcessor<WaveformAnalysisSinc>();
// Misc
AppendProcessor<CountProc>();
AppendProcessor<PruneProc>();
Expand Down
1 change: 1 addition & 0 deletions src/daq/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ add_library(daq OBJECT
src/WaveformPrep.cc
src/WaveformAnalyzerBase.cc
src/WaveformAnalysisLognormal.cc
src/WaveformAnalysisSinc.cc
)

# Set our include directories
Expand Down
70 changes: 70 additions & 0 deletions src/daq/include/RAT/WaveformAnalysisSinc.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
////////////////////////////////////////////////////////////////////
/// \class RAT::WaveformAnalysisSinc
///
/// \brief Interpolate waveforms by convolution using sinc kernel
///
/// \author Yashwanth Bezawada <[email protected]>
/// \author Tanner Kaptanoglu <[email protected]>
/// \author Ravi Pitelka <[email protected]>
/// \author James Shen <[email protected]>
///
/// REVISION HISTORY:\n
/// 25 Nov 2024: Initial commit
///
/// \details
/// This class provides full support for interpolating
/// the waveforms by convolution using a tapered sinc (tsinc)
/// kernel. Based on the implementation of the existing
/// WaveformAnalysis class and WaveformAnalysisLognormal class.
////////////////////////////////////////////////////////////////////
#ifndef __RAT_WaveformAnalysisSinc__
#define __RAT_WaveformAnalysisSinc__

#include <TObject.h>

#include <RAT/DB.hh>
#include <RAT/DS/DigitPMT.hh>
#include <RAT/Digitizer.hh>
#include <RAT/Processor.hh>
#include <RAT/WaveformAnalyzerBase.hh>
#include <cmath>
#include <vector>

namespace RAT {

class WaveformAnalysisSinc : public WaveformAnalyzerBase {
public:
WaveformAnalysisSinc() : WaveformAnalysisSinc(""){};
WaveformAnalysisSinc(std::string config_name) : WaveformAnalyzerBase("WaveformAnalysisSinc", config_name) {
Configure(config_name);
};
virtual ~WaveformAnalysisSinc(){};
void Configure(const std::string &config_name) override;
virtual void SetD(std::string param, double value) override;

// Interpolate the digitized waveform by convolution using a sinc kernel
std::vector<double> convolve_wfm(const std::vector<double> &wfm, const std::vector<double> &kernel);
void InterpolateWaveform(const std::vector<double> &voltWfm);

protected:
// Digitizer settings
DBLinkPtr fDigit;

// Analysis constants
double fFitWindowLow;
double fFitWindowHigh;

// Coming from WaveformPrep
double fDigitTimeInWindow;

// Fit variables
double fFitTime;
double fFitCharge;
double fFitPeak;

void DoAnalysis(DS::DigitPMT *pmt, const std::vector<UShort_t> &digitWfm) override;
};

} // namespace RAT

#endif
110 changes: 110 additions & 0 deletions src/daq/src/WaveformAnalysisSinc.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
#include <TF1.h>
#include <TH1D.h>
#include <TMath.h>

#include <RAT/Log.hh>
#include <RAT/WaveformAnalysisSinc.hh>

#include "RAT/DS/DigitPMT.hh"
#include "RAT/DS/WaveformAnalysisResult.hh"
#include "RAT/WaveformUtil.hh"

namespace RAT {
void WaveformAnalysisSinc::Configure(const std::string& config_name) {
try {
fDigit = DB::Get()->GetLink("DIGITIZER_ANALYSIS", config_name);
fFitWindowLow = fDigit->GetD("fit_window_low");
fFitWindowHigh = fDigit->GetD("fit_window_high");
} catch (DBNotFoundError) {
RAT::Log::Die("WaveformAnalysisSinc: Unable to find analysis parameters.");
}
}

void WaveformAnalysisSinc::SetD(std::string param, double value) {
if (param == "fit_window_low") {
fFitWindowLow = value;
} else if (param == "fit_window_high") {
fFitWindowHigh = value;
} else {
throw Processor::ParamUnknown(param);
}
}

void WaveformAnalysisSinc::DoAnalysis(DS::DigitPMT* digitpmt, const std::vector<UShort_t>& digitWfm) {
double pedestal = digitpmt->GetPedestal();
if (pedestal == -9999) {
RAT::Log::Die("WaveformAnalysisSinc: Pedestal is invalid! Did you run WaveformPrep first?");
}
// Convert from ADC to mV
std::vector<double> voltWfm = WaveformUtil::ADCtoVoltage(digitWfm, fVoltageRes, pedestal = pedestal);
fDigitTimeInWindow = digitpmt->GetDigitizedTimeNoOffset();

if (std::isinf(fDigitTimeInWindow) || fDigitTimeInWindow < 0) {
return;
}

InterpolateWaveform(voltWfm);

DS::WaveformAnalysisResult* fit_result = digitpmt->GetOrCreateWaveformAnalysisResult("Sinc");
fit_result->AddPE(fFitTime, fFitCharge, {{"peak", fFitPeak}});
}

std::vector<double> WaveformAnalysisSinc::convolve_wfm(const std::vector<double>& wfm,
const std::vector<double>& kernel) {
int n = wfm.size();
int m = kernel.size();
std::vector<double> result(n + m - 1, 0.0); // Size of the output
// Perform the convolution
for (int i = 0; i < n; ++i) {
for (int j = 0; j < m; ++j) {
result[i + j] += wfm[i] * kernel[j];
}
}
return result;
}

void WaveformAnalysisSinc::InterpolateWaveform(const std::vector<double>& voltWfm) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a couple issues with the name of this function, as well as accommodating documentations.

  1. No "interpolation" is actually performed here. Interpolation would suggest that we are going in between data samples. As far as I can tell, this is not happening. What is happening is a FIR convolution / filtering. So it should probably named as such.
  2. Charge and time calculation is actually done at the end of this method. This is not obvious that this is happening.

Copy link
Author

Choose a reason for hiding this comment

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

We are interpolating the waveform by convolving the waveform with the tsinc kernel! The convolution introduces 7 new points between two sampled data points. This is done in the convolve_wfm function. Sorry for the confusion with the function names. I will update the function names to better reflect what they are doing.

// Fit range
double bf = fDigitTimeInWindow - fFitWindowLow;
double tf = fDigitTimeInWindow + fFitWindowHigh;

// Check the fit range is within the digitizer window
bf = (bf > 0) ? bf : 0;
tf = (tf > voltWfm.size() * fTimeStep) ? voltWfm.size() * fTimeStep : tf;

// Get samples values within the fit range
std::vector<double> fit_wfm;
int sample_low = static_cast<int>(floor(bf / fTimeStep));
int sample_high = static_cast<int>(floor(tf / fTimeStep));
sample_high = std::min(static_cast<int>(voltWfm.size()) - 1, sample_high);
for (int j = sample_low; j < sample_high + 1; j++) {
fit_wfm.push_back(voltWfm[j]);
}

// Calculating tapered sinc kernel
std::vector<double> tsinc_kernel;
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this kernel calculation is independent of the input waveform parameters. Meaning that the kernel calculation should be placed in a static function, and should only be computed once, not during every single DoAnalysis call.

Furthermore, currently it seems all kernel parameters are hard-coded. I would suggest one of the following two changes:

  1. Expose relevant parameters (for example, ones that adjust the Fourier-Domain bandwidth of the sinc kernel) as a ratdb table entry
  2. convert this kernel calculation to a pre-calculated array, placing it in a ratdb entry.
    While option 2 may seem inferior at first glance, it would actually make this fitter a general "convolution" analyzer. I am not sure how useful this would be since the charge integration is quite simple, but perhaps people would want to run kernels with better time-domain behaviors.

int N = 8; // number of interpolated points per data point
double T = 30.0; // tapering constant
for (int k = -48; k < 49; k++) {
double val = k * 3.1415 / (float)N;
Copy link
Contributor

Choose a reason for hiding this comment

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

should use c++ style casting (static_cast in this case) instead of c-style casting. ditto for all cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

plus, N is only used as a float. Perhaps it should just be defined as a float.

Also, if this kernel is not refactored in a separate scope, please rename the single-letter variables to make them more searchable.

double sinc = sin(val);
if (k == 0)
sinc = 1.0;
else
sinc /= val;
tsinc_kernel.push_back(sinc * exp(-pow(k / T, 2)));
}

// Interpolated waveform
std::vector<double> interp_wfm = convolve_wfm(fit_wfm, tsinc_kernel);
std::pair<int, double> peakSampleVolt = WaveformUtil::FindHighestPeak(interp_wfm);

fFitPeak = peakSampleVolt.second;
fFitTime = bf + peakSampleVolt.first * fTimeStep / (float)N;
Copy link
Contributor

Choose a reason for hiding this comment

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

My apologies if I'm missing the math here. But this line seems to be indexing an upsampled waveform (therefore dividing sampling period by N) but I don't think any upsampling is done here (the convolution output waveform is N+M-1).

Again, I could be misunderstanding what you're trying to do. It would be nice if there's a brief explanation of the math being done as a writeup somewhere:)

Copy link
Author

Choose a reason for hiding this comment

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

You are correct. I went back and looked at the code. There is an error in my implementation while convolving the kernel and the sampled waveform. I will redo the code and push the changes.

fFitCharge = 0;
for (auto& vlt : interp_wfm) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do not use auto, unless we are looping through maps with pairs being the type.

fFitCharge += WaveformUtil::VoltagetoCharge(vlt, fTimeStep / (float)N, fTermOhms); // in pC
}
}

} // namespace RAT
Loading