Adding IDEA_o1_v02 and its vertex detector #273

armin-ilg · 2023-05-26T06:45:44Z

BEGINRELEASENOTES

Adding the IDEA_o1_v02 detector in key4hep, for the start just the vertex detector (vertex barrel + outer barrel) and the machine components, some place holder detectors otherwise.

ENDRELEASENOTES

To do's still:

Remove overlap between vertex support (imported from CAD) and vertex barrel
Cross check again all vertex dimensions
More detailed implementation of barrel (individual ARCADIA chips in vertex barrel, individual ATLASPix3 in outer barrel)
Add README.md

Longer term:

Add vertex digitisation (Armin)
Add silicon wrapper with timing information (Armin)
Add DRC, drift chamber, muon chambers

andresailer · 2023-05-26T13:20:32Z

What are these "*.swp" binary files?
Could you flatten the history please? I.e.: No merge from lcgeo/master commits, please use rebase.

BrieucF

Hi @armin-ilg ! Thanks! Could you

move everything from FCCee/compact/FCCee_IDEA_o1_v01/ to FCCee/compact/IDEA/IDEA_o1_v01
Add a README.md under detector/tracker explaining briefly what the new detector builders do together with a link to a recent reference (a talk is ok) describing the detector we are trying to model? You can just add two "entries" in the README to describe the two new builders, more will come to describe the other ones.

detector/tracker/ZPlanarTracker_geo.cpp

detector/tracker/VertexBarrel_IDEA_o1_v01_geo.cpp

detector/other/SingleShape_geo.cpp

BrieucF · 2023-06-22T08:37:25Z

Can you add a test for the newly introduced detectors, as in https://github.com/key4hep/k4geo/blob/master/lcgeoTests/CMakeLists.txt#L67 ?

FCCee/compact/IDEA/IDEA_o1_v01/Beampipe_o1_v01.xml

detector/tracker/ZPlanarTracker_geo.cpp

BrieucF · 2023-06-28T13:12:42Z

Hi Armin, I did not anticipate that we were going to have scripts and utilities here, but it is perfectly ok. I would then just propose a modification in the folder organization because the compact folder should contain compact files, not script. Could you organize things with: FCCee/IDEA/compact where you put only the xmls, FCCee/IDEA/scripts where you put the scripts that are specific to IDEA (maybe passing the specific compact file as an argument instead of hard coding it so that it will work also for o1_v02)? Regarding very generic utils like e.g. overlap.mac, I would put them in the k4geo/utils but it has also been introduced in PR#271 so you could just remove it from your PR and take the one introduced there.

andresailer · 2023-06-29T13:29:04Z

But now we have https://github.com/armin-ilg/lcgeo/blame/master/detector/tracker/ZPlanarTracker_geo.cpp

FCCee/compact/IDEA/IDEA_o1_v01/HOMAbsorber_o1_v01.xml

armin-ilg · 2023-07-17T14:23:40Z

What are these "*.swp" binary files?

Could you flatten the history please? I.e.: No merge from lcgeo/master commits, please use rebase.

.swp files are deleted.
I created a flattened branch (https://github.com/armin-ilg/lcgeo/tree/flattened), I can use that one for the merge request if you want.

jmcarcell · 2023-07-17T14:33:00Z

It's better to continue here where there is already discussion. For flattening you can always force push to your branch and then not use merge and use rebase. At the end the commits will be squashed (if the squash button is pressed, which should) and then it doesn't really matter if the history is linear or not since it will just be another commit on top of the current one

armin-ilg · 2023-07-17T15:02:16Z

Can you add a test for the newly introduced detectors, as in https://github.com/key4hep/k4geo/blob/master/lcgeoTests/CMakeLists.txt#L67 ?

Done

…1_v01_geo module

…nts, ability to tilt whole modules.

… show them)

… volumes not yet

…ake good assembly structure

…ll there)

…refore

…im and fccRec_e4h with vertex digitisation, but no vertices reco'd yet, bad hierarchy again

andresailer · 2023-09-05T15:37:44Z

FCCee/IDEA/compact/IDEA_o1_v01/Vertex_IDEA_o1_v01.xml

+    <detectors>
+        <detector id="0" name="VTXIBSupport" type="DD4hep_TestShape_Creator">
+            <check>
+                <shape type="CAD_MultiVolume" ref="${K4GEO}/share/k4geo/FCCee/IDEA/compact/IDEA_o1_v01/VertexSupport_o1_v01.obj" unit="mm"/>


You are saying, you cannot use a relative path here?

Yes, just using ref="VertexSupport_o1_v01.obj" will only work if it's run inside of the FCCee/IDEA/compact/IDEA_o1_v01 folder. I can open a ticket in DD4hep for that if you think this should be fixed (would be useful I think)

Yes please open an issue. We must not depend on running in the same folder or having the full path.

Done: AIDASoft/DD4hep#1166

FCCee/IDEA/compact/IDEA_o1_v01/Beampipe_o1_v01.xml

…, change name when they differ)

lcgeoTests/CMakeLists.txt

… The test will only run once the IDEA folder is in the nightly Key4hep software stack

lcgeoTests/CMakeLists.txt

…lume names can be assigned. Furthermore changed DetElements in barrel constructor so that they can be properly accessed in k4RecTracker

… This is applied in the IDEA vertex detector. Other than this, also the width of the sensor support in the vertex barrel was corrected to be only 4.4 mm wide (instead of 8.8)

…, only end-cap missing

…ponents. Multiple components can be added per stave and the name will be used to name the corresponding assembly. Furthermore fixed the sensitive/periphery regions in VTXIB layers 2 and 3 to be correct

… end-cap.

andresailer · 2023-09-28T09:22:27Z

This is going to conflict with #282 for the IDEA_o1_v01

BrieucF · 2023-09-28T09:49:45Z

Actually there is conflict with #282 since both are putting thing in IDEA_o1_v01. I would propose to rename the version introduced here as IDEA_o1_v02 because #282 migrates the IDEA detector as it was in FCCDetectors (and it makes sense to call this version o1_v01) while you are using a modified vertex detector + modified beampipe. What do you think?

When this PR will be merged (I hope soon), I will start adding to IDEA_o1_v02 the detailed drift chamber and the new muon chambers!

armin-ilg · 2023-09-28T15:02:34Z

Hi. So I have done the last additions to the IDEA vertex. It's now also possible to do some very simple (and by far not final) digitisation using k4RecTracker (my fork is here) without needing to use the k4MarlinWrapper.

The IDEA vertex detector is now quite accurately described. In comparison to the MDI note, also the end-of-stave/ladder structures have been implemented. Only the large holder structure for the disks is missing.

Anything else that needs to be changed @andresailer ?

andresailer · 2023-09-28T15:05:22Z

What Brieuc posted above again.

andresailer · 2023-09-28T15:08:55Z

Also check yourself why the tests are failing please.

…pport as it will fail the test, since the CAD file cannot be found. After the IDEA detector is in a nightly release, then the CAD file will be found and we can include these lines in the xml again

armin-ilg · 2023-09-29T07:32:09Z

Also check yourself why the tests are failing please.

Done. I commented the vertex inner barrel support for the moment, as this would prevent the test from passing.
(see previous problem)

Yes, just using ref="VertexSupport_o1_v01.obj" will only work if it's run inside of the FCCee/IDEA/compact/IDEA_o1_v01 folder. I can open a ticket in DD4hep for that if you think this should be fixed (would be useful I think)

armin-ilg · 2023-09-29T07:34:13Z

What Brieuc posted above again.

@andresailer : So if I understand Brieuc's comment correctly

Actually there is conflict with #282 since both are putting thing in IDEA_o1_v01. I would propose to rename the version introduced here as IDEA_o1_v02 because #282 migrates the IDEA detector as it was in FCCDetectors (and it makes sense to call this version o1_v01) while you are using a modified vertex detector + modified beampipe. What do you think?
When this PR will be merged (I hope soon), I will start adding to IDEA_o1_v02 the detailed drift chamber and the new muon chambers!

this means that we can merge this PR now and then the other open PR will build on this IDEA version, right? So we can go ahead and merge this PR from your side @BrieucF ?

BrieucF · 2023-09-29T09:19:55Z

What Brieuc posted above again.

@andresailer : So if I understand Brieuc's comment correctly

Actually there is conflict with #282 since both are putting thing in IDEA_o1_v01. I would propose to rename the version introduced here as IDEA_o1_v02 because #282 migrates the IDEA detector as it was in FCCDetectors (and it makes sense to call this version o1_v01) while you are using a modified vertex detector + modified beampipe. What do you think?
When this PR will be merged (I hope soon), I will start adding to IDEA_o1_v02 the detailed drift chamber and the new muon chambers!

this means that we can merge this PR now and then the other open PR will build on this IDEA version, right? So we can go ahead and merge this PR from your side @BrieucF ?

Hi Armin, sorry I was not clear. No, we should first rename the folder IDEA_o1_v01 --> IDEA_o1_v02, rename IDEA_o1_v01.xml --> IDEA_o1_v02.xml and modify the relevant paths in affected scripts. Then it is good for merging on my side.

armin-ilg · 2023-10-02T13:57:57Z

What Brieuc posted above again.

@andresailer : So if I understand Brieuc's comment correctly

Actually there is conflict with #282 since both are putting thing in IDEA_o1_v01. I would propose to rename the version introduced here as IDEA_o1_v02 because #282 migrates the IDEA detector as it was in FCCDetectors (and it makes sense to call this version o1_v01) while you are using a modified vertex detector + modified beampipe. What do you think?
When this PR will be merged (I hope soon), I will start adding to IDEA_o1_v02 the detailed drift chamber and the new muon chambers!

this means that we can merge this PR now and then the other open PR will build on this IDEA version, right? So we can go ahead and merge this PR from your side @BrieucF ?

Hi Armin, sorry I was not clear. No, we should first rename the folder IDEA_o1_v01 --> IDEA_o1_v02, rename IDEA_o1_v01.xml --> IDEA_o1_v02.xml and modify the relevant paths in affected scripts. Then it is good for merging on my side.

Ah, sorry for the confusion. Changed now!
(the Silicon wrapper will be added in a future PR, although I have a preliminary version already)

armin-ilg marked this pull request as ready for review May 26, 2023 06:52

BrieucF reviewed Jun 12, 2023

View reviewed changes

detector/tracker/ZPlanarTracker_geo.cpp Outdated Show resolved Hide resolved

BrieucF reviewed Jun 13, 2023

View reviewed changes

detector/tracker/VertexBarrel_IDEA_o1_v01_geo.cpp Outdated Show resolved Hide resolved

armin-ilg force-pushed the master branch from ac6cc46 to 21b60df Compare June 22, 2023 07:08

BrieucF reviewed Jun 22, 2023

View reviewed changes

detector/other/SingleShape_geo.cpp Outdated Show resolved Hide resolved

andresailer reviewed Jun 22, 2023

View reviewed changes

FCCee/compact/IDEA/IDEA_o1_v01/Beampipe_o1_v01.xml Outdated Show resolved Hide resolved

andresailer reviewed Jun 26, 2023

View reviewed changes

detector/tracker/ZPlanarTracker_geo.cpp Outdated Show resolved Hide resolved

armin-ilg force-pushed the master branch from 5661e65 to a8ac0c3 Compare June 29, 2023 14:33

andresailer reviewed Jul 6, 2023

View reviewed changes

FCCee/compact/IDEA/IDEA_o1_v01/HOMAbsorber_o1_v01.xml Outdated Show resolved Hide resolved

armin-ilg mentioned this pull request Jul 17, 2023

Adding possibility to estimate material budget not only in eta, but a… HEP-FCC/k4SimGeant4#42

Merged

Armin Fehr added 13 commits July 17, 2023 17:25

Adding module functionality to ZPlanarTracker_geo

8d465f5

Leaving ZPlanarTracker as it is but instead make a new VertexBarrel_o…

c00f642

…1_v01_geo module

Added ability to have flex and support consisting of multiple compone…

640115c

…nts, ability to tilt whole modules.

First implementation of IDEA end-cap and adaption of barrel

f259579

Version of Barrel with correct sensitive surfaces (use teveDisplay to…

fa1d847

… show them)

Improved structure of barrel detectors (layer - ladder+sensors)

6061af7

Progress: The surface arrows are now displayed in the endcap, but the…

6d6af32

… volumes not yet

Whole barrel is properly visible in teveDisplay, but don't mange to m…

e69ee1d

…ake good assembly structure

Also endcap shows all volumes in teveDisplay, but bad hierarchy

012de49

Endcap and barrel now run through ddsim overlap checker (overlaps sti…

e165a35

…ll there)

IDEA Vertex working with ddsim, some overlaps still to be understood

cc2656d

Not working teveDisplay, endcap digitisation not working probably the…

dee4d5a

…refore

Fixed endcap, all surfaces visible in teveDisplay, managed to run dds…

1a22125

…im and fccRec_e4h with vertex digitisation, but no vertices reco'd yet, bad hierarchy again

andresailer reviewed Sep 5, 2023

View reviewed changes

FCCee/IDEA/compact/IDEA_o1_v01/Beampipe_o1_v01.xml Outdated Show resolved Hide resolved

Renaming beampipe to match with CLD beampipe (identical at the moment…

d66a05d

…, change name when they differ)

andresailer reviewed Sep 6, 2023

View reviewed changes

lcgeoTests/CMakeLists.txt Outdated Show resolved Hide resolved

Fixing test and have correct name for beampipe also in main xml file.…

3693833

… The test will only run once the IDEA folder is in the nightly Key4hep software stack

andresailer reviewed Sep 6, 2023

View reviewed changes

lcgeoTests/CMakeLists.txt Outdated Show resolved Hide resolved

armin-ilg mentioned this pull request Sep 6, 2023

Allow to give CAD files with relative paths similar to importing xml files (DDCAD) AIDASoft/DD4hep#1166

Closed

Multiple properties in one SET_TESTS_PROPERTIES instance

cfcaf04

andresailer self-assigned this Sep 6, 2023

armin-ilg added 6 commits September 20, 2023 14:13

Moved volume creation within main part of program, so that correct vo…

11d1105

…lume names can be assigned. Furthermore changed DetElements in barrel constructor so that they can be properly accessed in k4RecTracker

Added functionality to have end of stave structures in vertex barrel.…

6a4c536

… This is applied in the IDEA vertex detector. Other than this, also the width of the sensor support in the vertex barrel was corrected to be only 4.4 mm wide (instead of 8.8)

Added end-of-stave structures or proxies as well for the outer barrel…

7646e21

…, only end-cap missing

Making proxy end-of-stave structure smaller to not have overlaps

13554da

Unified description of readout and support structures, now called com…

c07d81c

…ponents. Multiple components can be added per stave and the name will be used to name the corresponding assembly. Furthermore fixed the sensitive/periphery regions in VTXIB layers 2 and 3 to be correct

Fixing volIDs in barrel, adding end-of-stave structures in barrel and…

aa26ee6

… end-cap.

Correcting material in beam pipe

eafb798

Fixing test and for the moment comment out the vertex inner barrel su…

406a3f5

…pport as it will fail the test, since the CAD file cannot be found. After the IDEA detector is in a nightly release, then the CAD file will be found and we can include these lines in the xml again

Changing to IDEA_o1_v02

d149e7e

andresailer changed the title ~~[WIP] Adding IDEA detector to k4geo~~ Adding IDEA detector to k4geo Oct 3, 2023

andresailer changed the title ~~Adding IDEA detector to k4geo~~ Adding IDEA_o1_v02 and its vertex detector Oct 3, 2023

andresailer merged commit 1dfca23 into key4hep:master Oct 3, 2023
4 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding IDEA_o1_v02 and its vertex detector #273

Adding IDEA_o1_v02 and its vertex detector #273

armin-ilg commented May 26, 2023 •

edited by andresailer

Loading

andresailer commented May 26, 2023 •

edited

Loading

BrieucF left a comment

BrieucF commented Jun 22, 2023

BrieucF commented Jun 28, 2023

andresailer commented Jun 29, 2023

armin-ilg commented Jul 17, 2023

jmcarcell commented Jul 17, 2023 •

edited

Loading

armin-ilg commented Jul 17, 2023

andresailer Sep 5, 2023

armin-ilg Sep 6, 2023

andresailer Sep 6, 2023

armin-ilg Sep 6, 2023

andresailer commented Sep 28, 2023

BrieucF commented Sep 28, 2023

armin-ilg commented Sep 28, 2023

andresailer commented Sep 28, 2023

andresailer commented Sep 28, 2023

armin-ilg commented Sep 29, 2023

armin-ilg commented Sep 29, 2023

BrieucF commented Sep 29, 2023

armin-ilg commented Oct 2, 2023

Adding IDEA_o1_v02 and its vertex detector #273

Adding IDEA_o1_v02 and its vertex detector #273

Conversation

armin-ilg commented May 26, 2023 • edited by andresailer Loading

andresailer commented May 26, 2023 • edited Loading

BrieucF left a comment

Choose a reason for hiding this comment

BrieucF commented Jun 22, 2023

BrieucF commented Jun 28, 2023

andresailer commented Jun 29, 2023

armin-ilg commented Jul 17, 2023

jmcarcell commented Jul 17, 2023 • edited Loading

armin-ilg commented Jul 17, 2023

andresailer Sep 5, 2023

Choose a reason for hiding this comment

armin-ilg Sep 6, 2023

Choose a reason for hiding this comment

andresailer Sep 6, 2023

Choose a reason for hiding this comment

armin-ilg Sep 6, 2023

Choose a reason for hiding this comment

andresailer commented Sep 28, 2023

BrieucF commented Sep 28, 2023

armin-ilg commented Sep 28, 2023

andresailer commented Sep 28, 2023

andresailer commented Sep 28, 2023

armin-ilg commented Sep 29, 2023

armin-ilg commented Sep 29, 2023

BrieucF commented Sep 29, 2023

armin-ilg commented Oct 2, 2023

armin-ilg commented May 26, 2023 •

edited by andresailer

Loading

andresailer commented May 26, 2023 •

edited

Loading

jmcarcell commented Jul 17, 2023 •

edited

Loading