Skip to content

Conversation

@akioogawa
Copy link
Contributor

Briefly, what does this PR introduce?

Fix photon count inconsistency for ScFI

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No

Does this PR change default behavior?

No

akioogawa and others added 30 commits May 1, 2025 15:00
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
… 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
Co-authored-by: Dmitry Kalinkin <[email protected]>
Co-authored-by: Dmitry Kalinkin <[email protected]>
Co-authored-by: Dmitry Kalinkin <[email protected]>
Updated sampling fraction and formatting for configuration parameters.
/home/runner/work/EICrecon/EICrecon/src/detectors/FEMC/FEMC.cc:105:39: note: insert an explicit cast to silence this issue
  105 |          .numEffectiveSipmPixels    = totalPixel},
      |                                       ^~~~~~~~~~
      |                                       static_cast<unsigned long long>( )
@akioogawa
Copy link
Contributor Author

Also applying resolution smearing (1.0 + eResRel) factor should be before SiPM saturation. I thought we talked about it? Cannot recall maybe you rejected for a reason. At least for FEMC, edep should be modifyed by (1.0 + eResRel) factor, then go through all photon statistics and SiPM sauration.

wdconinc and others added 3 commits November 18, 2025 16:52
This PR applies the include-what-you-use fixes as suggested by
https://github.com/eic/EICrecon/actions/runs/19474751836.
Please merge this PR into the branch `fEMCal_photon_count`
to resolve failures in PR #2190.

Auto-generated by [create-pull-request][1]

[1]: https://github.com/peter-evans/create-pull-request

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

wdconinc and others added 2 commits November 19, 2025 09:54
This PR applies the include-what-you-use fixes as suggested by
https://github.com/eic/EICrecon/actions/runs/19505021244.
Please merge this PR into the branch `fEMCal_photon_count`
to resolve failures in PR #2190.

Auto-generated by [create-pull-request][1]

[1]: https://github.com/peter-evans/create-pull-request

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot dismissed their stale review November 19, 2025 15:12

No Clang-Tidy warnings found so I assume my comments were addressed

wdconinc and others added 2 commits November 19, 2025 14:22
This PR applies the include-what-you-use fixes as suggested by
https://github.com/eic/EICrecon/actions/runs/19508711009.
Please merge this PR into the branch `fEMCal_photon_count`
to resolve failures in PR #2190.

Auto-generated by [create-pull-request][1]

[1]: https://github.com/peter-evans/create-pull-request

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent photon count in fEMCal

4 participants