Trapping correction module and app - #184
Conversation
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)
| m_Name = "Trapping Correction"; // - correcting energies for charge trapping (by Sophie); | ||
|
|
||
| // Set the XML tag --- has to be unique --- no spaces allowed | ||
| m_XmlTag = "TrappingCorrection"; |
There was a problem hiding this comment.
Let's be consistent with XML tag naming here (see #193)
| m_XmlTag = "TrappingCorrection"; | |
| m_XmlTag = "XmlTagTrappingCorrection"; |
There was a problem hiding this comment.
I'm actually not seeing this convention in the depth or energy calibration modules? It looks like the XML tag is just the name of the module
There was a problem hiding this comment.
This is tackled in #197 once it is merged
There was a problem hiding this comment.
Once I change the name of the xml tag, nuclearizer can no longer recognize the module. Is there somewhere else in assembly I need to update the xml tag? I was looking but I couldn't find anything
There was a problem hiding this comment.
This will affect reading in a nuclearizer config file (*.cfg). In those, you might need to update all occurences of TrappingCorrection with XmlTrappingCorrection.
There was a problem hiding this comment.
So either update the .nuclearizer.cfg in your home directory, or the respective .cfg that you load when running nuclearizer.
| 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).
updated parameter file and module for multi detector trapping correction
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