Skip to content

Conversation

@mhkim-anl
Copy link
Contributor

@mhkim-anl mhkim-anl commented Sep 12, 2025

Briefly, what does this PR introduce?

This PR is to plug in PulseCombiner and PulseNoise for BIC. For the PulseCombiner, pulses from different particles were combined for each SiPM channel. For the PulseNoise, a specific pedestal distribution was reproduced so that it gives the same pedestal mean and sigma when the amplitudes are measured every 25 ns. To reproduce the specific pedestal model, an offset was added to the PulseNoiseConfig.h. Since the order of the arguments in the FalphaNoise was swapped, it was modified.

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?

Does this PR change default behavior?

This PR applies the include-what-you-use fixes as suggested by
https://github.com/eic/EICrecon/actions/runs/17684050541.
Please merge this PR into the branch
`2075-plugging-in-pulsecombiner-and-pulsenoise-for-bic`
to resolve failures in PR #2076.

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>
@mhkim-anl mhkim-anl marked this pull request as draft September 24, 2025 19:13
@mhkim-anl
Copy link
Contributor Author

Based on the Slides presented at the simulation meeting, the followings were updated.

  • In the SimCalorimeterHitProcessor, branches that are unlikely to be used are filled with 0.
  • EcalBarrelScFiRawHits and EcalBarrelScFiRawHitAssociations are excluded from the default output.
  • EcalBarrelScFi{P, N}Pulses, EcalBarrelScFi{P, N}CombinedPulses, and EcalBarrelScFi{P, N}CombinedPulsesWithNoise are added to the default output.

In the slides, it is expected that this PR reduces the data size even though the BIC pulse data types are added, but I added one more pulse data, EcalBarrelScFi{P, N}CombinedPulses compared to the slides just in case. In this case, the final data size is expected to be comparable with that without this PR.

@mhkim-anl mhkim-anl marked this pull request as ready for review September 25, 2025 21:34
Copy link
Member

@veprbl veprbl left a comment

Choose a reason for hiding this comment

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

LGTM

@veprbl veprbl added this pull request to the merge queue Oct 17, 2025
Merged via the queue into main with commit a04e5e8 Oct 17, 2025
129 checks passed
@veprbl veprbl deleted the 2075-plugging-in-pulsecombiner-and-pulsenoise-for-bic branch October 17, 2025 16:08
"EcalBarrelScFiPAttenuatedHitContributions",
"EcalBarrelScFiNAttenuatedHits",
"EcalBarrelScFiNAttenuatedHitContributions",
"EcalBarrelScFiRawHits",
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the reason for removing EcalBarrelScFiRawHits here???

Copy link
Contributor

Choose a reason for hiding this comment

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

Re-added in #2140.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry for the late reply. I was attending the DNP. I thought we use EcalBarrelScFiRecHits rather than EcalBarrelScFiRawHits when we analyze the eicrecon output. I also thought EcalBarrelScFiRawHits would be replaced by a new RawHGCROCHit once the digitization algorithm is implemented. So I excluded it from the default output to reduce the size of the BIC output.

github-merge-queue bot pushed a commit that referenced this pull request Oct 17, 2025
### Briefly, what does this PR introduce?
This PR re-adds the EcalBarrelScFiRawHits and associations to the
output, after they were removed in #2076.

### What kind of change does this PR introduce?
- [x] Bug fix (issue: regression when used outputs were removed)
- [ ] New feature (issue #__)
- [ ] Documentation update
- [ ] Other: __

### 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?
Restore outputs.
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.

Plugging in PulseCombiner and PulseNoise for BIC

4 participants