-
Notifications
You must be signed in to change notification settings - Fork 41
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
Implement the new detector parameters #552
Labels
Milestone
Comments
EDIT: update the comparison for
|
7 tasks
This seems to be a bug in the comparison script (in addition to a geometry mismatch). The physical start should not be Edit: it affects multiple detectors. |
Yes, it seems an issue in the template. |
This was referenced Oct 9, 2023
Chao1009
added this to the Update Compact Files According to the 2023.09 Detector Parameters milestone
Oct 10, 2023
This was referenced Oct 12, 2023
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 12, 2023
### Briefly, what does this PR introduce? Update the template for generating a detector parameter table from simulation constants. The new template will be created based on the latest detector parameter table. (https://eic.jlab.org/Geometry/Detector/local/D/DetectorParameterTable-20230927.csv) It only focuses on the comparable columns, so some columns such as "comments" will be dropped. This PR partially resolves #552 ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [x] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [x] 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 --------- Co-authored-by: Chao Peng <cpeng@anl.gov> Co-authored-by: Wouter Deconinck <wdconinc@gmail.com>
7 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Dec 9, 2023
### Briefly, what does this PR introduce? This PR related to the issue #552, and implements small changes in the Barrel ECal geometry to make it more realistic. The following changes has been implemented: - SciFi/Pb z_min and z_max (and accordingly the full length) has been implemented according to https://eic.jlab.org/Geometry/Detector/Detector-20231031150001.html. - The total depth (envelope) of the calorimeter has been increased to 38 cm (from fixed 35 cm). 38 cm envelope is the correct one and fits both the SciFi and the 3 cm back Al plate. - ~~In every AstroPix stave a carbon fiber slice has been added to mimic a bit better the material of the staves~~ ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue #552) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [x] 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? Calorimeter (SciFi/Pb matrix) length, offset, and z_min z_max are in agreement with the geometry database: Output of `npdet_info dump $DETECTOR_PATH/epic_craterlake.xml | grep EcalBarrel` ``` EcalBarrel_AvailThickness = 35.000 = EcalBarrelRegion_thickness - EcalBarrel_Support_thickness EcalBarrel_Calorimeter_length = 440.000 = EcalBarrel_Calorimeter_zmax + EcalBarrel_Calorimeter_zmin EcalBarrel_Calorimeter_offset = -38.750 = (EcalBarrel_Calorimeter_zmax - EcalBarrel_Calorimeter_zmin)/2.0 EcalBarrel_Calorimeter_zmax = 181.250 = min(181.25*cm, EcalBarrelForward_zmax) EcalBarrel_Calorimeter_zmin = 258.750 = min(258.75*cm, EcalBarrelBackward_zmax) ``` Now calorimeter has proper depth (together with back plate) ![SciFi part of the Calo. 12th layer has now proper width.](https://github.com/eic/epic/assets/33816222/70384133-39c5-4fb6-a19c-c5c5fbc1093b) ![Zoom on AstroPix part of the Calo](https://github.com/eic/epic/assets/33816222/f11b6e6d-8a06-430b-93f0-b3ff8c5273e4) Not included in this PR because of the problem with overlaps Below you can see the Carbon Fiber slice added to each stave. ![AstroPix part of the sector](https://github.com/eic/epic/assets/33816222/d3edbe40-14eb-46de-9908-454f46d5d30d) ![Zoom on one layer with modules](https://github.com/eic/epic/assets/33816222/a127bc45-79ca-479f-887f-a325e1a263b9) --------- Co-authored-by: Maria <zurek@anl.gov>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
The current simulation geometry implementation is inconsistent with the latest detector parameter table.
det: https://eic.jlab.org/Geometry/Detector/local/D/DetectorParameterTable-20230927.csv
sim: https://eic.github.io/epic/artifacts/DetectorParameterTable/epic_craterlake.csv
Describe the solution you'd like
Update the parameter table template and the compact files to make them as consistent as possible. (some mismatches are from geometry and some are from table generation)
Some of the updates may require changes in geometry plugins. These are supposed to be coordinated work with the corresponding DSCs.
Describe alternatives you've considered
No.
Additional context
Corresponding DSCs need to be informed and confirm the changes before merging PRs.
The text was updated successfully, but these errors were encountered: