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

Improvements to vertex/silicon wrapper #396

Merged
merged 3 commits into from
Sep 25, 2024
Merged

Conversation

armin-ilg
Copy link
Contributor

@armin-ilg armin-ilg commented Sep 24, 2024

BEGINRELEASENOTES

  • No more warnings in silicon wrapper
  • Improvements in vertex builder printouts
  • Adding all materials of beam pipe also to material_plots_2D.py, as without having the beam pipe enabled, the vertex material budget estimation will fail.
  • Changed paths to .stl files in vertex to use https://fccsw.web.cern.ch/fccsw/filesForSimDigiReco/IDEA/IDEA_o1_v03/STL_FILES/ (still commented out due to overlaps)
    ENDRELEASENOTES

@BrieucF: As you requested I fixed the warnings in the silicon wrapper.

P.s:
I don't know how to squash all the commits, sorry that this is such a mess

@andresailer
Copy link
Contributor

Can you stop touching your branch for 5 minutes?

… in vertex detector constructors, changed paths of .stl filesin vertex (still commented out due to overlaps)
…ngs as input to be ignored and list of substrings in materials to be removed
@BrieucF
Copy link
Contributor

BrieucF commented Sep 25, 2024

Shall we merge this before or after #392?

@tmadlener
Copy link
Contributor

There will most likely be a conflict either way. So, I don't have any strong preferences.

@andresailer
Copy link
Contributor

Merging this, applying a styler and then cherry-picking the diff from #392 is probably easier than the other way around, and these changes here are only feature changes, so would not be nice to punish Armin :)

@BrieucF BrieucF merged commit 0b59886 into key4hep:main Sep 25, 2024
6 checks passed
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