Skip to content
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

Include dual-readout endcap tubes calorimeter in IDEA_o2 xml file #411

Merged
merged 26 commits into from
Dec 6, 2024

Conversation

lopezzot
Copy link
Member

@lopezzot lopezzot commented Nov 22, 2024

BEGINRELEASENOTES

  • A new subdetector for the dual-readout capillary-tubes-based endcap calorimeter is included as discussed here
  • As INFN recently changed the baseline of the IDEA detector, a new IDEA_o2 XML file is created accordingly. It includes the new DREndcapTubes subdetector, changes the solenoid inner radius and removes the preshower. XML files for IDEA_o2 dimensions and materials are included as well. IDEA_o2 will be completed with the new crystal em calorimeter and the dual-readout capillary-tubes-based barrel calorimeter.
  • A new example example/SteeringFile_IDEA_o2_v01.py is included to run a simulation with the IDEA_o2 concept.
  • A new SDAction (DRTubesSDAction) is included and applied to the DREndcapTubes subdetector via regexSD in example/SteeringFile_IDEA_o2_v01.py, this speeds up the event rate w.r.t. previous dual-readout calorimetry simulations and reduces the memory fooprint as explained here

ENDRELEASENOTES

Hello,

This PR should be the starting point for the new IDEA concept as proposed by INFN. The inner part of IDEA up to the Silicon Wrapper is not touched. The solenoid inner radius is increased and between the SiWrapper and the solenoid a new crystal dual-readout em calorimeter will be included. When PR #406 is merged crystals will be included in the IDEA_o2 XML file. The preshower is removed. The hadronic endcap calorimeter is the new DREndcapTubes which uses the INFN Hidra-like technology. A new PR will be created with the barrel hadronic calorimeter. The muon system inner radius will have to be changed accordingly and for the moment it is commented-out in the IDEA_o2 XML file.

Testing:

  • I produced 1000 10 GeV electrons events shot directly in the middle of the endcap calorimeter. They were simulated on lxplus9 at a rate of 0.8 s/evt with no warnings. Distributions from the calohits look good.
  • Thanks to regexSD the memory footprint of this partial IDEA_o2 configuration is 4.5 Gb (previously it was 15 Gb).
  • I run an overlap check with a 10 um tolerance and no overlaps were detected. However, checking for overlaps over 30M tubes takes forever, so the check was done without fibers inside the calorimeter towers and each standalone tower was checked for overlaps independently.

To do in the next PRs:

  • In case this PR is merged the next steps for IDEA_o2 are to include the dual-readout em crystal calorimeter from Segmented Crystal ECAL (SCEPCal) addition #406 and the dual-readout capillary-tubes barrel calorimeter, to apply DRTubesSDAction to the barrel calorimeter and to adjust the muon system dimensions to avoid overlaps with the barrel calorimeter at theta=90 degree.

Please let me know if you agree with this strategy for IDEA option2. Thanks!

@BrieucF
Copy link
Contributor

BrieucF commented Nov 25, 2024

Hi Lorenzo, great!

Can you add a test similar to https://github.com/key4hep/k4geo/blob/main/test/CMakeLists.txt#L93 ?

Since #406 is orthogonal, I think we can proceed with this PR regardless and add the crystals when ready.

@lopezzot
Copy link
Member Author

lopezzot commented Nov 25, 2024

@BrieucF I added a test for IDEA_o2_v01, it takes about 211 s on lxplus9 machines largely dominated by the construction time of the DREndcapTubes subdetector.

I agree this PR can proceed regardless of the crystal one.

@atolosadelgado
Copy link
Collaborator

Hi @lopezzot

thanks for this PR, looks great!

I have only 2 questions:

  • I understand that the SD action is setup in the steering file, but that file is not provided when running the test. Do you think is it worth it to use the steering file when running the test?
  • I have run the simulation using different particles, and no tracking warnings are thrown. When plotting the hit pattern radius vs z, there is a pattern, is it ok? The picture below show the hit pattern in the endcap for kaons at 50GeV
    DR_endcap_kaons

Thank you for your time.
Alvaro
PS: while running your geometry, I spotted a minor bug in DDSim, so double thanks.

@BrieucF
Copy link
Contributor

BrieucF commented Nov 26, 2024

Can you also explain the hot spot at z ~2800? I naively would not expect that with 50 GeV K_longs...

@lopezzot
Copy link
Member Author

lopezzot commented Nov 26, 2024

Hello @atolosadelgado and @BrieucF, thank you for your time and very good questions.

  1. Yes, I think it is worth using the steering file in the test. The only caveat is that regexSD takes a while to perform searches over so many volumes so the test time will increase. I will add a commit with this change and comment on the test time.

  2. About hit positions (x,y,z). They are not the global position of the step inside a fiber but the position of the inner tip of the fiber where the step is taking place. If a track crosses the same fiber twice a single hit will be created with (x,y,z) equal to the the fiber inner tip and the signal of the corresponding fiber will be incremented twice. That is why there is a spot at 280 cm which is where most of fiber tips are (the endcap itself starts at 280 cm). The patterns Alvaro sees are due to the fact that new fibers (with shorter lengths) are included along the side of the tower (trapezoid) as soon as towers open up. Therefore some fibers have an inner tip not starting at 280 cm. These fibers are positioned in rows and columns around the tower at given depths along towers and this is the reason why some clusters appear in the scatter plot. In other words, not every fiber starts at 280 cm but those that don't are not randomly positioned, so some spots are visible even for them. This is my understanding for the moment and looks consistent with the geometry implemented (of course experience may prove me wrong).

EDIT: I added a commit below to use the steering file in the test. It takes about 350 s on lxplus9, I believe it is still acceptable.

@lopezzot
Copy link
Member Author

lopezzot commented Nov 28, 2024

Hi @andresailer, @atolosadelgado, @BrieucF while I was trying to add a "system" id for the DREndcapTubes subdetector I think I spotted a problem with DD4hep Assembly volumes. In my code, I use an assembly volume to place inside it phi-slices volumes. I do this placement in a for loop over j and assign j as geant4 copynumber and dd4hep volID to the volumes placed inside the Assembly https://github.com/lopezzot/k4geo/blob/lp-idea_o2/detector/calorimeter/dual-readout-tubes/src/DREndcapTubes_o1_v01.cpp#L167.
However, when I get the Geant4 copynumber in the DRTubesSDAction code for these phi-slices volumes, the copynumber returned is not the one I set in the DD4hep geometry code. This reminds me of this problem AIDASoft/DD4hep#1277 (if i am not wrong) related to an LHCb issue with reproducibility of IDs. As far as I understand, while DD4hep converts the Assembly geometry node in geant4 node, copynumbers are internally changed and this is a problem because I need geant4 copynumbers to recreate the 64-bits volume IDs in my DRTubesSDAction code. What do you think? should I open an issue for this? thank you.

@andresailer
Copy link
Contributor

should I open an issue for this? thank you.

Yes, please open an issue for this in DD4hep.

@BrieucF
Copy link
Contributor

BrieucF commented Nov 29, 2024

Hello @atolosadelgado and @BrieucF, thank you for your time and very good questions.

1. Yes, I think it is worth using the steering file in the test. The only caveat is that regexSD takes a while to perform searches over so many volumes so the test time will increase. I will add a commit with this change and comment on the test time.

2. About hit positions (x,y,z). They are not the global position of the step inside a fiber but the position of the inner tip of the fiber where the step is taking place. If a track crosses the same fiber twice a single hit will be created with (x,y,z) equal to the the fiber inner tip and the signal of the corresponding fiber will be incremented twice. That is why there is a spot at 280 cm which is where most of fiber tips are (the endcap itself starts at 280 cm). The patterns Alvaro sees are due to the fact that new fibers (with shorter lengths) are included along the side of the tower (trapezoid) as soon as towers open up. Therefore some fibers have an inner tip not starting at 280 cm. These fibers are positioned in rows and columns around the tower at given depths along towers and this is the reason why some clusters appear in the scatter plot. In other words, not every fiber starts at 280 cm but those that don't are not randomly positioned, so some spots are visible even for them. This is my understanding for the moment and looks consistent with the geometry implemented (of course experience may prove me wrong).

EDIT: I added a commit below to use the steering file in the test. It takes about 350 s on lxplus9, I believe it is still acceptable.

Thanks for the explanation. Some more questions. Do you also store the single hits into CaloHitContributions? Do you set the position to the "real" hit position (instead of the fiber front) for those? Will this be enough to perform studies on possible longitudinal information from timing?

@lopezzot
Copy link
Member Author

Thanks for the explanation. Some more questions. Do you also store the single hits into CaloHitContributions? Do you set the position to the "real" hit position (instead of the fiber front) for those? Will this be enough to perform studies on possible longitudinal information from timing?

Good point. For the moment I do not save calohitcontributions. Our experience with test-beam data, and standalone simulations of the 4pi geometry, show that having only the fibre tip position is already good enough for the vast majority of studies we want to do. Also note that in IDEA_o2 crystals will be positioned before this calo, and the need for longitudinal segmentation in the fiber-dr calorimeter is then reduced. In any case, for longitudinal studies it would certainly be possible to add calohitcontributions and save the true hit position and timing of each hit inside fibers. I believe a dedicated PR with some full-sim studies could be open just after this one.

@BrieucF
Copy link
Contributor

BrieucF commented Nov 29, 2024

Thanks for the explanation. Some more questions. Do you also store the single hits into CaloHitContributions? Do you set the position to the "real" hit position (instead of the fiber front) for those? Will this be enough to perform studies on possible longitudinal information from timing?

Good point. For the moment I do not save calohitcontributions. Our experience with test-beam data, and standalone simulations of the 4pi geometry, show that having only the fibre tip position is already good enough for the vast majority of studies we want to do. Also note that in IDEA_o2 crystals will be positioned before this calo, and the need for longitudinal segmentation in the fiber-dr calorimeter is then reduced. In any case, for longitudinal studies it would certainly be possible to add calohitcontributions and save the true hit position and timing of each hit inside fibers. I believe a dedicated PR with some full-sim studies could be open just after this one.

Alright, for another PR then! Thanks

@BrieucF
Copy link
Contributor

BrieucF commented Dec 5, 2024

Can you update the branch? Are we good to go?

@lopezzot
Copy link
Member Author

lopezzot commented Dec 5, 2024

Can you update the branch? Are we good to go?

This PR AIDASoft/DD4hep#1362 fixes the problem I mentioned above. Therefore for what concerns my code, I believe we are good to go. Just two last questions:

  1. Is it ok if I clang-format this code before merging? have you got a .clang-format file example in case or I can go with mine?
  2. I have to rebase before merging, right?

@BrieucF
Copy link
Contributor

BrieucF commented Dec 5, 2024

Can you update the branch? Are we good to go?

This PR AIDASoft/DD4hep#1362 fixes the problem I mentioned above. Therefore for what concerns my code, I believe we are good to go. Just two last questions:

1. Is it ok if I clang-format this code before merging? have you got a .clang-format file example in case or I can go with mine?

2. I have to rebase before merging, right?
  1. Yes it is ok for me. You could potentially use this one: https://github.com/key4hep/k4FWCore/blob/main/.clang-format . I do not have a strong opinion, maybe @andresailer has ?
  2. Yes, please rebase, I think only test/CMakeList.txt was touched both here and in the PR you are missing (https://github.com/key4hep/k4geo/pull/409/files#diff-33394812ba204689144fd2f80832db83853ba1cb32403edb4e15fe4893e675fd)

Add geometry for the dual-readout tubes-based endcap calorimeter (taken
from https://github.com/lopezzot/DREndcapTube/tree/v0.3).
Add CMake code to compile geometries under
detector/calorimeter/dual-readout-tubes/*, the barrel geometry will be
added there.
Add xml description file for IDEA_o2_v01. It includes the
dual-readout-tubes endcap calorimeter. As this option will include a
crystal em-section, the preshower is removed from the xml file.
Add DRTubesSDAction as in lopezzot/DREndcapTube v0.3. Comment out
RunAction and EventAction classes which are not needed for execution
inside k4geo. File name changed from DREndcapTubesSDAction to
DRTubesSDAction as this SDAction code will be used for DR barrel
detector using capillary tubes.
Rename DREndcapTubesSDData to DRTubesSDData as DRTubesSDAction will be
common for endcap and barrel tubes calorimeter. Also typedef to
DRTubesSDAction.
RunAction and EventAction will not be used in key4geo simulation. They
were used in standalone simulation to produced plain root files with
G4AnalysisManager.
Same as SteeringFile_IDEA_o1_v03.py file but lines for DRC have been
removed and those specific for the DRTubes subdetector have been added
(e.g. regexSD).
Remove elements.xml file, it is identical to the one sourced from
IDEA_o1_v03 directory. Also fix typos in IDEA//README.md IDEA_o2
description.
Add XML file for materials definition for IDEA_o2. At the moment is
identical to IDEA_o1_v03 but new materials (e.g. crystal ones) will be
included. Also remove Vacuum and Air material definition from
DREndcapTubes_o1_v01 XML file.
lopezzot and others added 18 commits December 5, 2024 17:52
Add XML file with dimensions for IDEA_o2. Same as
DectDimensions_IDEA_o1_v01 but Fiber dual-readout calorimeter dimensions
have been removed and dual-readout-tubes endcap calorimeter dimensions
have been added. Also DREndcapTubes DetID is set in
DectDimensions_IDEA_o2_v01. In future the preshower dimensions should be
removed as well.
Move vis attributes definition for dual-readout-tubes endcap calorimeter
inside DectDimensions_IDEA_o2_v01. Remove vis attributes from dr fiber
calorimeter.
Fix bug in SteeringFile_IDEA_o2_v01.py, the DRTubesSDAction was assigned
to every calorimeter, it should be assigned only to DREndcapTubes (and
later on to the dual-readout-tubes barrel calorimeter).
The calo.filter="edep0" in SteeringFile_IDEA_o2_v01.py did not let the
DRTubesSDAction be executed on opticalphotons which did not deposit
energy in fibers. However the SDAction must be executed on those steps
as they are counted to create the Cherenkov signal and they are killed
to speedup the simulation. Replace to calo.filter=None. Also set the
physicsList as FTFP_BERT. And the beam to 10 GeV e- shot directly into
the endcap calorimeter to monitor the time per event.
According to indications from the IDEA members, the new dimensions for
the calorimeter are: inner radius 2.8 m and length 1.8 m. Also changed
the number of rotations around Z-axis to 72.
The new em-crystal section will be included inside the solenoid. The
solenoid dimensions are changes to inner radius 2.5 m and outer radius
2.8 m.
IDEA_o2 will not include the preshower, therefore I remove it from
IDEA_o2_v01.xml file. Also the preshower dimensions are removed from
DectDimensions_IDEA_o2_v01.xml.
The DREndcapTubes phi air staves reached the beam pipe (y=0), this was
clearly causing an overlap with the compensating solenoid. To fix this a
new variable set the starting y of the phi air staves to 22 cm from the
z-axis. This commit was checked for overlaps with a tolerance of 10 um
(fibers not included in towers) and no overlaps were detected.
To monitor the event rate 1000 events are needed to get stable results.
With this example we have around 0.8-1.0 s/evt for 10 GeV e- showeing in the
endcap calorimeter.
Adding a CMake test for IDEA_o2_v01. ctest -R t_test_IDEA_o2_v01 takes
about 211 s on lxplus9 machines. This test spotted one "Error" keyword
due to anticlockwise ordering of G4GenericTrap constructor edges (they
were internally reordered by root). To fix it I reordered them
clockwise.
Fix indentation of SteeringFile_IDEA_o2_v01.py file causing an error
against pre-commit hook.
Change cmake test for IDEA_o2 in order to use the IDEA_o2 example
steering file. This test takes about 350 s on lxplus9 machines and
correctly fills DREndcapTubes calo hit with 1 event.
Fix comment for unit translation from Geant4 units to EDM4hep units.

Co-authored-by: Andre Sailer <[email protected]>
Add a comment to explain how photoelectrons are computed from Monte
Carlo information: energy deposited in S fibers and number of C photons
trapped in fibers.
Add comment about using our custom Birks' Law implementation in the
SDAction. In particular to avoid potential double application of Birks'
correction if the Geant4StepHandler is used on this subdetector.
Add "system" id to DREndcapTubes xml file and assign it to the highest
volume in the geometry node. While recreating the volumeID in the
DRTubesSDAction, "system" id is set as in DectDimensions_IDEA_o2_v01.xml
file. In both xml files add a comment on ids to prevent unwanted
modifications.
Add a comment in DRTubesSDAction to check that 64-bits volumeIDs created
with g4-copynumbers are identical to the original DD4hep volumeIDs.
clang-format .cpp and .hh files for DREndcapTubes subdetector. I checked
that this formatting does not change physics results.
@lopezzot
Copy link
Member Author

lopezzot commented Dec 5, 2024

Formatted and rebased.

@lopezzot
Copy link
Member Author

lopezzot commented Dec 6, 2024

Just a small correction to the previous testing: the memory footprint is now around 2.5 Gb, that's because the tower granularity was increased from 36 to 72 rotations around the beam axis. Less unique tubes are now inserted inside a phi-slice and more unique tubes are replicated in phi-slices, therefore the memory footprint is reduced.
Can we proceed with merging?

@BrieucF
Copy link
Contributor

BrieucF commented Dec 6, 2024

Yes, I was leaving some time for people to comment about the clang formatting. Looks like there is no complain.

Thanks Lorenzo!

@BrieucF BrieucF merged commit 37e6562 into key4hep:main Dec 6, 2024
6 checks passed
@lopezzot lopezzot deleted the lp-idea_o2 branch December 6, 2024 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants