framework icon indicating copy to clipboard operation
framework copied to clipboard
verified publisher icon rest-for-physics

Adding generic methods for signal analysis inside framework

Open juanangp opened this issue 2 years ago • 9 comments

juanangp Large: 785 Powered by Pull Request Badge

New namespace for generic signal analysis inside framework, it implements different methods:

  • Different methods for baseline calculation
  • Signal smoothing
  • Methods for signal analysis (risetime, width, points over threshold, triple max, integral,...)
  • Methods for signal fitting (gaus, landau, aget)

Related PR:

  • https://github.com/rest-for-physics/rawlib/pull/100
  • https://github.com/rest-for-physics/connectorslib/pull/27
  • https://github.com/rest-for-physics/detectorlib/pull/75

juanangp avatar Feb 15 '23 17:02 juanangp

I understand it is much cleaner to create this dedicated class, however, I still believe the right place for it is at the rawlib.

Only in rawlib it will make sense to calculate baselines, or does it make sense in any other existing or future library?

Thus, I suggest to rename this class to TRestRawSignalAnalysis and place it at rawlib.

jgalan avatar Feb 21 '23 06:02 jgalan

If new types (other than Short_t) are introduced for rawsignals, those types should be added somehow also inside rawlib.

Please, share in case that you foresee another use-case where you will need a library for raw signal processing that it is outside rawlib.

jgalan avatar Feb 21 '23 06:02 jgalan

Please, share in case that you foresee another use-case where you will need a library for raw signal processing that it is outside rawlib.

I think this topic is discussed on https://github.com/rest-for-physics/framework/issues/353. Personally, I think that signal analysis methods should be generic and doesn´t have to be linked to any library. From my understanding rawlib holds raw signal data but it doesn't mean that all the signal processing has to be done in this library. For instance, I was attemting to perform the analysis of the smoothed signal which was difficult with the current code design, this is actually implemented on under a related PR https://github.com/rest-for-physics/connectorslib/pull/27. In addition detectorlib currently holds several methods for signal processing, despite signal smoothing or baseline calculations, there are several methods such as GetMaxLandau(), GetMaxGauss() or GetMaxAget() that are used inside TRestDetectorSignalToHitsProcess. On the other hand, I don't see any issue to perform the signal analysis on a ´TRestDetectorEvent´.

juanangp avatar Feb 21 '23 09:02 juanangp

Should TRestSignalAnalysis be renamed to something different such as TRestTimeSignalAnalysis, TRestPulseShapeAnalysis or TRestWaveFormAnalysis? TRestSignal it is quite generic and it might be used in an statistical case, such as it could be the case of using TRestSignal and TRestBackground components.

jgalan avatar Apr 03 '23 17:04 jgalan

Should TRestSignalAnalysis be renamed to something different such as TRestTimeSignalAnalysis, TRestPulseShapeAnalysis or TRestWaveFormAnalysis? TRestSignal it is quite generic and it might be used in an statistical case, such as it could be the case of using TRestSignal and TRestBackground components.

Indeed, it can be renamed since it can be misleading with new developments, I vote for TRestPulseShapeAnalysis

juanangp avatar Apr 03 '23 18:04 juanangp

@lobis and @jgalan can you approve or give me feedback in the related PR? I cannot merge otherwise.

  • https://github.com/rest-for-physics/rawlib/pull/100
  • https://github.com/rest-for-physics/connectorslib/pull/27
  • https://github.com/rest-for-physics/detectorlib/pull/75

juanangp avatar Apr 21 '23 08:04 juanangp

If I remember correctly you cannot write the implementation of the template methods separately in the cxx file, unless you #include this cxx file like a .h file. I don't know why it works as the pipeline is green...

nkx111 avatar May 15 '23 10:05 nkx111

If I remember correctly you cannot write the implementation of the template methods separately in the cxx file, unless you #include this cxx file like a .h file. I don't know why it works as the pipeline is green...

As far as I know, template specializations can be defined in the source file https://stackoverflow.com/questions/115703/storing-c-template-function-definitions-in-a-cpp-file I think it is cleaner to do the implementation inside the source file. The code compiles and works properly.

juanangp avatar May 15 '23 18:05 juanangp

Note that the following submodule PR needs to be merged together with this one:

  • https://github.com/rest-for-physics/rawlib/pull/100
  • https://github.com/rest-for-physics/connectorslib/pull/27
  • https://github.com/rest-for-physics/detectorlib/pull/75

juanangp avatar May 17 '23 09:05 juanangp