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

refactor: geant4 converters #2048

Merged
merged 10 commits into from
May 8, 2023

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Apr 20, 2023

split off changes from my calorimeter demo. this changes most of the shared pointers to unique pointers in the converter because we use unique pointers downstream sometimes. and unique pointers can be upgraded to shared pointers directly. it also includes a generalization of the material conversion and transform conversion

update and summary

  • switched from unique_ptr consistently to shared_ptr after discussion
  • added Geant4VolumeConverter
  • added transform translation for G4 phyiscal volumes
  • added generic material translation

@andiwand andiwand added the Component - Plugins Affects one or more Plugins label Apr 20, 2023
@andiwand andiwand added this to the next milestone Apr 20, 2023
@andiwand andiwand requested a review from asalzburger April 20, 2023 17:16
@andiwand andiwand changed the title refactor: geant4 convertes refactor: geant4 converters Apr 20, 2023
@codecov
Copy link

codecov bot commented Apr 20, 2023

Codecov Report

Merging #2048 (6c36307) into main (2e45ac2) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2048      +/-   ##
==========================================
- Coverage   49.40%   49.39%   -0.01%     
==========================================
  Files         434      434              
  Lines       24984    24984              
  Branches    11524    11526       +2     
==========================================
- Hits        12343    12341       -2     
  Misses       4486     4486              
- Partials     8155     8157       +2     
Impacted Files Coverage Δ
Core/include/Acts/Detector/DetectorVolume.hpp 74.28% <ø> (ø)
Core/src/Detector/DetectorVolume.cpp 49.36% <ø> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

github-actions bot commented Apr 20, 2023

📊 Physics performance monitoring for 6c36307

Full report
Seeding: seeded, truth estimated, orthogonal
CKF: seeded, truth smeared, truth estimated, orthogonal
IVF: seeded, truth smeared, truth estimated, orthogonal
Ambiguity resolution: seeded, orthogonal
Truth tracking
Truth tracking (GSF)

Vertexing

Vertexing vs. mu
IVF seeded

IVF truth_smeared

IVF truth_estimated

IVF orthogonal

Seeding

Seeding seeded

Seeding truth_estimated

Seeding orthogonal

CKF

CKF seeded

CKF truth_smeared

CKF truth_estimated

CKF orthogonal

Ambiguity resolution

seeded

Truth tracking (Kalman Filter)

Truth tracking

Truth tracking (GSF)

Truth tracking

Copy link
Contributor

@asalzburger asalzburger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am in principle ok with the PR, just wanted to cross-check on a few points.

@andiwand
Copy link
Contributor Author

@asalzburger implemented your suggested changes

@andiwand andiwand requested a review from asalzburger April 28, 2023 09:00
Copy link
Contributor

@asalzburger asalzburger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go, questions have been addressed.

@asalzburger asalzburger merged commit 9a19dbc into acts-project:main May 8, 2023
@andiwand andiwand deleted the refactor-geant4-convertes branch May 9, 2023 09:08
@paulgessinger paulgessinger modified the milestones: next, v26.0.0 May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Plugins Affects one or more Plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants