Bug fix on Barrel TOF sensors having incorrect segmentation. #834
Conversation
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
… two halve size sensors side-by-side. If sensor sizes are allowed to vary, the x-offset value in CartesianGridXY will either messed up the segmentation of full or halve size sensors. You can't satisfy both since the offset value depend on sensor size. The only solution is to only use sensors of the same size across the entire BTOF.
for more information, see https://pre-commit.ci
github-actions
bot
added
topic: barrel
|
Just a note, I think you can solve it simpler, with multi segmentation defining which offset is used. That would be also future-proof if you need to reposition half sensors with respect to the others. epic/compact/tracking/mpgd_barrel.xml Lines 126 to 138 in 2e62d15 |
|
Thanks @veprbl for your suggestion. Yeah, that implementation make more sense. I did not know that segmentation class exist. Now I've learnt something new. I will update my pull request soon using MultiSegmentation. |
… the BTOF cell position bug.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
The pull request is updated to using MultiSegmentation. |
6 tasks
github-merge-queue bot
pushed a commit
to eic/EICrecon
that referenced
this pull request
Mar 4, 2025
#1750) …on changes. ### Briefly, what does this PR introduce? The segmentation class of BTOF changes from CartesianGridXY to MultiSegmentation since eic/epic#834. Therefore, we need to remove the segmentation class restriction on LGADChargeSharing class. ### What kind of change does this PR introduce? - [x ] Bug fix (issue #__) - [ ] 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? No.
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?
We realize that the cell positions are not in the correct places on some BTOF sensors. Specifically, the cell center for the half-sensors immediately to the left and right of the central gap for the engagement ring are all incorrect. This issue is caused by the segmentation class (CartesianGridXY) applying the wrong offset on the half-sensors. Unfortunately, I do not know of a way to apply a different offset value just for the half-sensor while keeping the original value for the normal sensor.
This pull request resolves that issue by making all sensors half-sized. A full sensor is simply two half-sensors placed side-by-side.
Please read our discussion on the ePIC AC-LGAD TOF weekly meeting for details: https://indico.bnl.gov/event/27008/contributions/103896/attachments/60237/103439/BTOF_geometry_bug_EIC_EPIC.pdf
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?
BTOF now uses CartesianStripX for segmentation rather than CartesianGridXY. However, the field names for cellID remains unchanged since I renamed the identifies for CartesianStripX to match the behavior of CartesianGridXY.
Does this PR change default behavior?
No.