Trapping correction module and app - #184
Conversation
|
Can we also remove |
fhagemann
left a comment
fhagemann
left a comment
There was a problem hiding this comment.
Here is my feedback from just looking at the code.
I will also try to compile and run both the module and the app to provide more feedback on that as well! :)
|
The example file in 
|
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
fhagemann
left a comment
fhagemann
left a comment
There was a problem hiding this comment.
Final code-style review, will give it a run to see that it compiles.
But then this should be good to go (tackling reading trapping parameters for multiple detectors in a follow-up PR)
| cout << "\n" << m_XmlTag << " --- " << titleLabel << " ---" << endl; | ||
| cout << " Centroid (Mu) : " << mu << " keV" << endl; | ||
| cout << " Fitted Gaussian FWHM : " << fwhm << " keV" << endl; | ||
| cout << " Direct Histogram FWHM: " << directFWHM << " keV" << endl; |
There was a problem hiding this comment.
I use this output to evaluate how well the trapping correction is working. If I change the verbosity to info, the comand line becomes overloaded (I believe w info from the depth cal) and nuclearizer crashes. I agree this info doesn't need to be repeated so I can change the verbosity for the following comments but I do need it read out somewhere.
I think we probably need to clean up the outputs for the info verbosity right now because right now it's outputting so much info that I can't receive the info I'm trying to get from the trapping correction.
There was a problem hiding this comment.
@zoglauer maybe we can introduce the chatty verbosity from megalib also to nuclearizer, and move some of the info messages to chatty?
There was a problem hiding this comment.
yeah that could be a good idea!
fhagemann
left a comment
fhagemann
left a comment
There was a problem hiding this comment.
This would be good from my side. Just gathering here the open TODOs (for follow-up PRs):
- Allow for a charge trapping parameters file for multiple detectors, at not just one
- Think about flushing output to
c_Infowithout overloading the output - Maybe find an alternative to
Interpolateto avoid code duplication (maybe from an existing package or another module --> the depth calibration also used some sort of interpolation algorithm to interpolate the CTD-to-depth spline).
3 participants
Contains a new module for applying depth-based charge trapping correction to individual hit energies along with modules with GUI options and GUI expos. The branch also includes an app for characterizing trapping with Cs-137 data. The trapping correction module requires a csv parameter file as input:
detector_0_trapping_parameters.csv
This file can also be found in resource/dee