-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added SimDRCalorimeterHit for dual-readout #380
Open
wonyongc
wants to merge
4
commits into
key4hep:main
Choose a base branch
from
wonyongc:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
161daf6
Added SimDRCalorimeterHit for dual-readout
wonyongc e685ea6
Make sure to only run tests with compatible root versions (#381)
tmadlener f18a2f2
Simpler DR Calorimeter Hits
wonyongc 2bf4a94
Merge remote-tracking branch 'upstream/main'
wonyongc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why produced photons? Why not recorded photons? Similar to having the avg arrival time of photons recorded in the time fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a SIM level object, I guess we should leave the possibility to treat effects such as efficiency in a subsequent digitizer (though it is indeed sometimes interesting to leak part of the digitization in the SDAction)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then the tAvg should also come from the digitiser and not be calculated here already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some input from side. Our idea here is to have a first functional simulation which is a sufficiently good representation of the detector performance. The definition of the technology is still being matter of R&D and thus we are not ready yet to work on the digitization part. At this first stage, in our opinion, the most efficient approach (also from the computational time point of view) is to factorize the simulation of energy deposition part from that of optical photon tracing, SiPM and electronics.
From the former we need:
For what concerns the time stamp it is often sufficient to save the average time of all hits inside a certain calorimetric cell (possibly weighed with the energy of each hit). This is for our calorimeter a sufficiently good approximation of the time information we can extract with a realistic system (possibly after applying a resolution smearing).
It is to early to have a realistic implementation of the digitizer step in my opinion because of two reasons:
Concerning having separate collections for the scintillation and cherenkov photons this is also possible I believe. However, the difference with respect to the dual-readout fiber calorimeter is that in our case we have a complete duplications of hits since the cherenkov photons are always produced in the same volumes as that of other hits. Having two separate collections looked like redundant (I don't know about the impact on data size). For each hit in a certain calorimetric cell we always have both scintillation and cherenkov non-zero signals. If we use two separate collections we will have to match one-by-one all hits at the reconstruction level.