Update FEMC for new geometry: #1848
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adding switch between Homogeneous and ScFi geometry implementations based on xml file loaded - For Homogenous, we keep enrgy smeasring as is. SF=1.0 - For ScFi, hits from fibers are summed to a tower and no enrgy smearing applied. SF=0.03. Added option to put SiPM saturation to CalorimeterHitDigi - Specify 2 new parameters : totalPixel and nPhotonPerGeV - default for totalPixel is 0, which case no attenuation is applied For FEMC for both geometry models, SiPM saturation is ON by default - Use "-PFEMC:SiPMSaturation=OFF" to turn it off
github-actions
bot
added
topic: calorimetry
|
Please address the conflicts by performing a git rebase (preferably). As long as there are conflicts the automatic checks can't even start to run. |
|
Done |
|
As I understand, FEMC insert wil not be build. So it was removed from geometry and here FEMC factories. How can I remove it from auto checks? |
Removing a detector would be something to do in a separate pr. |
… to Homogeneous model (see https://www.star.bnl.gov/~akio/epic/reco/index.html) - Adding ScFi model's resolution smearing of 2.2% constant term based on high energy end (see https://www.star.bnl.gov/~akio/epic/reco/index.html) - Changing ADC threshold from 2 (~9MeV which was too low) to 3 (~15MeV as intended) - Rename an unfortunate variable name
for more information, see https://pre-commit.ci
veprbl
reviewed
Jun 3, 2025
| // Note: both adc and tdc values must be positive numbers to avoid integer wraparound | ||
| unsigned long long adc = | ||
| std::max(std::llround(ped + edep * corrMeanScale_value * (1.0 + eResRel) / | ||
| std::max(std::llround(ped + edep * corrMeanScale_value * (1.0 + eResRel) * saturation / |
veprbl
reviewed
Jun 3, 2025
|
|
||
| int SiPMSaturation = 1; | ||
| std::string opt("ON"); | ||
| app->SetDefaultParameter("FEMC:SiPMSaturation", opt, "Turn ON(default) or OFF SiPM Saturation"); |
There was a problem hiding this comment.
We should avoid adding options inside plugins, when algorithm option would apply to different detectors. See how its made into algorithm parameter in #1064 .
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Briefly, what does this PR introduce?
Following Forward EM calorimeter geometry update:
eic/epic#855
this provides switch between Homogeneous and ScFi geometry implementations based on xml file loaded
Added an option to put SiPM saturation to CalorimeterHitDigi
See following links for some more details:
https://www.star.bnl.gov/~akio/epic/geometry/index.html
https://www.star.bnl.gov/~akio/epic/reco/index.html
https://www.star.bnl.gov/~akio/epic/hadron/index.html
What kind of change does this PR introduce?
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
I don't think so, unless one provides wrong config xml file (until sum check in place)
Does this PR change default behavior?
It does apply SiPM attenuation by default for FEMC (but not other calorimeters).