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

IDEA vertex and silicon wrapper for IDEA_o1_v03 #363

Merged
merged 184 commits into from
Sep 19, 2024

Conversation

armin-ilg
Copy link
Contributor

BEGINRELEASENOTES

  • Update of IDEA vertex, with the ability to use the ultra-light vertex concept in-situ.
  • No overlaps in all of vertex and silicon wrapper (not including the DDCAD imported vertex support and cooling cones yet), more performant silicon wrapper (only silicon wrapper barrel sensors are simplified)

ENDRELEASENOTES

Hi @BrieucF

Here finally is the vertex and silicon wrapper, as presented in the FCC Week. This version is very performant, even with the sensitive surface module running. I push it directly to IDEA_o1_v03, so we can close the old PR.

  • This version also works with iLCSoft reconstruction (I had some issues with the old version. Apparently all barrel layers must be in the same envelope and this envelope has to be continuous). The "CLD with IDEA vertex" version, that I used for impact parameter resolution studies, is not included here however.
  • I currently have the .obj files (other CAD format) directly in the repo. I see that for the MDI they are in an external place - shall I do the same?
  • I'll work on the updated Silicon Wrapper design (as presented in FCC Week) soon - but this version here is okay for the moment I think. I'll make another MR for that.

Is there something you would like to see changed? I'm doing some final tests to see whether iLCSoft reco really is still working.

Cheers,
Armin

Armin Fehr and others added 30 commits December 18, 2023 16:24
…im and fccRec_e4h with vertex digitisation, but no vertices reco'd yet, bad hierarchy again
…at latest beam pipe and other files are used. There's also still an overlap between the CAD-imported vertex support and the vertex barrel
…l and vertex disks, adding working DDCAD import of vertex inner barrel support (using material budget estimation from FCCSW not working yet though, need to use g4MaterialScan). Adding example on how I install k4geo locally (install.sh)
@armin-ilg
Copy link
Contributor Author

Fixed a last issue with ALLEGRO, now everything is final. No overlaps detected. Ready to merge @atolosadelgado @BrieucF @andresailer

@BrieucF
Copy link
Contributor

BrieucF commented Sep 18, 2024

Hi Armin, thanks! Two tests fail because of the renaming of the materials file under IDEA. Can you update those lines:

@BrieucF
Copy link
Contributor

BrieucF commented Sep 18, 2024

No overlaps detected

Did you also check ALLEGRO?

@BrieucF
Copy link
Contributor

BrieucF commented Sep 18, 2024

Another important thing: similarly to the CAD beampipe, the binary stl files are too big for a gitHub repository (this PR would increase the repo size by 70 MB). I propose to use the same trick as for the beampipe: download the files at build time with this command: https://github.com/key4hep/k4geo/blob/main/CMakeLists.txt#L224 . I don't think you need to place this inside an if statement since by default those files are used. But please, do check whether the file are there already (https://github.com/key4hep/k4geo/blob/main/CMakeLists.txt#L220) and download them only if they are not.

I have placed the stl files here: https://fccsw.web.cern.ch/fccsw/filesForSimDigiReco/IDEA/IDEA_o1_v03/STL_FILES/

Thanks!

@BrieucF
Copy link
Contributor

BrieucF commented Sep 18, 2024

Ha wait, sorry, I had not seen that the loading of the .stl files was commented out. This can therefore be handled in another PR. You can just remove the .stl from the PR then (and we should merge with "squash" to avoid having themin the commit history).

@armin-ilg
Copy link
Contributor Author

Hi Armin, thanks! Two tests fail because of the renaming of the materials file under IDEA. Can you update those lines:

Done, I also updated the vertex and silicon wrapper in those tests, hope that's okay

@armin-ilg
Copy link
Contributor Author

Another important thing: similarly to the CAD beampipe, the binary stl files are too big for a gitHub repository (this PR would increase the repo size by 70 MB). I propose to use the same trick as for the beampipe: download the files at build time with this command: https://github.com/key4hep/k4geo/blob/main/CMakeLists.txt#L224 . I don't think you need to place this inside an if statement since by default those files are used. But please, do check whether the file are there already (https://github.com/key4hep/k4geo/blob/main/CMakeLists.txt#L220) and download them only if they are not.

I have placed the stl files here: https://fccsw.web.cern.ch/fccsw/filesForSimDigiReco/IDEA/IDEA_o1_v03/STL_FILES/

Thanks!

Done, will later make another MR with the way you propose

@BrieucF
Copy link
Contributor

BrieucF commented Sep 19, 2024

Any further comment @andresailer ?

@andresailer
Copy link
Contributor

Any further comment @andresailer ?

No, all good from my side.

@BrieucF
Copy link
Contributor

BrieucF commented Sep 19, 2024

Wonderful! Thanks!

So, IDEA only misses a presampler now !! :-)

@BrieucF BrieucF merged commit 72c9480 into key4hep:main Sep 19, 2024
5 of 6 checks passed
@armin-ilg
Copy link
Contributor Author

Thanks for all the help, feedback and support!

@giovannimarchiori
Copy link
Contributor

Great @armin-ilg! Can you please close the PR superseded by this one?

@armin-ilg
Copy link
Contributor Author

Great @armin-ilg! Can you please close the PR superseded by this one?

Thanks for reminding me. Done

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.

6 participants