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

Implementing generic analysis methods inside detectorlib #75

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

juanangp
Copy link
Member

@juanangp juanangp commented Mar 31, 2023

juanangp Ok: 86 Powered by Pull Request Badge

Implementing generic methods inside detectorlib after being implemented here rest-for-physics/framework#379

Summary of changes:

  • TRestDetectorSignal implementing generic methods from TRestSignalAnalysis
  • TRestDetectorSignal moving to std iterators rather than custom ones
  • TRestDetectorSignalToHitsProcess implementing generic methods from TRestSignalAnalysis
  • TRestDetectorSignalToHitsProcess added new function GetHitZCoordinate to avoid code duplication
  • Removal of baseline calculation methods

Double_t GetStandardDeviation(Int_t startBin, Int_t endBin);
Double_t GetBaseLine(Int_t startBin, Int_t endBin);
Double_t GetBaseLineSigma(Int_t startBin, Int_t endBin, Double_t baseline = 0);
void CalculateBaseLineAndSigma(Int_t startBin, Int_t endBin, Double_t& baseline, Double_t& baseLineSigma);
Copy link
Member

@jgalan jgalan Mar 31, 2023

Choose a reason for hiding this comment

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

For me it makes no sense to have a baseline at the detector signal level. This is a stage where we have already applied data reduction. And in the best case reduced our waveforms into physical times and amplitudes.

It was kind of first approach to take all points from raw to detector signal. But we should move towards a reconstruction that reduces the data.

I understand the issue with short_t precision in rawlib, that's why I think having a std::vector <float_t> or even std::vector <double> implemented inside TRestRawSignal would solve this problem, so it this vector is not empty it means a process has filled it, and then processing will continue at float_t or double_t levels.

That way you leave also all time-signal processing routines contained within rawlib, which was one of the duties of the raw library. To compile all those codes used for time signal processing into a single entity.

Copy link
Member Author

Choose a reason for hiding this comment

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

These functions can be indeed deleted, however I don't know if it is a good idea to define a std::vector <float_t> inside rawlib since it could be misleading. Which data is supposse to hold? Data with baseline substracted, points over threshold or smoothed data?

There are as well signal processing functions that can be used in other context, e.g. tripleMax, GausFit and many others are currently implemented inside detectorlib (actually tripleMax was duplicated inside rawlib). But in principle one could try to perform a gaussian fit to a TRestRawSignalEvent.

I believe that to make a generic class or namespace inside framework for signal analysis or processing is the way to go...

@juanangp juanangp marked this pull request as ready for review April 3, 2023 17:46
@juanangp juanangp requested a review from a team April 3, 2023 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants