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

Moving to generic methods inside TRestRawSignal #100

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

Conversation

juanangp
Copy link
Member

@juanangp juanangp commented Feb 15, 2023

juanangp Ok: 80 Powered by Pull Request Badge

Using generic methods inside TRestRawSignal after being implemented inside framework rest-for-physics/framework#379

Summary of changes:

  • Implementing generic methods inside TRestRawSignal
  • Moving fPointsOverThreshold from std::vector<Int_t> to std::vector<std::pair<Float_t, Float_t> >
  • New function GetData which returns std::vector<Float_t> vector with the baseline substracted

@juanangp juanangp changed the title WIP: Moving to generic methods inside TRestRawSignal Moving to generic methods inside TRestRawSignal Apr 3, 2023
@juanangp juanangp marked this pull request as ready for review April 3, 2023 17:45
@juanangp juanangp requested review from lobis, jgalan, KonradAltenmueller and a team April 3, 2023 17:45
src/TRestRawSignal.cxx Outdated Show resolved Hide resolved
src/TRestRawSignal.cxx Outdated Show resolved Hide resolved
juanangp and others added 2 commits May 23, 2023 12:26
Co-authored-by: Luis Antonio Obis Aparicio <[email protected]>
Co-authored-by: Luis Antonio Obis Aparicio <[email protected]>
@jgalan
Copy link
Member

jgalan commented May 23, 2023

What about TRestRawSignal inheriting from TRestPulseShapeAnalysis?

@@ -56,7 +52,7 @@ class TRestRawSignal : public TObject {
TGraph* fGraph; //!

/// A std::vector containing the index of points that are identified over threshold.
std::vector<Int_t> fPointsOverThreshold; //!
std::vector<std::pair<Float_t, Float_t> > fPointsOverThreshold; //!
Copy link
Member

Choose a reason for hiding this comment

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

Better to have fPointsOverThreshold in correspondence with TRestRawSignal's data structure. x should be time bin(int) and y should be amplitude(unsigned short). Therefore the returned type should be std::vector<std::pair<Int_t, UShort_t> >

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I didn't notice this, I also think it would make more sense <Int_t, UShort_t>

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at the code fPointsOverThreshold corresponds to the points with the baseline substracted, so for me it doesn't make sense to return std::vector<std::pair<Int_t, Short_t> > since we will be loosing precision.

@juanangp
Copy link
Member Author

juanangp commented May 25, 2023

What about TRestRawSignal inheriting from TRestPulseShapeAnalysis?

I dont' think this would be a good idea since TRestRawSignal would be inheriting twice. Moreover, in the current inplementation TRestPulseShapeAnalysis is a namespace so it is not possible to inherit. Perhaps, we can discuss this topic in the meeting.

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.

5 participants