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

feat: particle killing 4 Geant4 and refactor user actions #2057

Merged
merged 12 commits into from
Apr 27, 2023

Conversation

benjaminhuth
Copy link
Member

Out Geant4 algorithm allows to pass std::vectors of user actions. However, multiple actions per type seem not to be supported by Geant4. This changes this to allow only one user action per type (run, event, particle, step).

I'm not sure if this was possible in former Geant4 versions, there is also a comment in the Geant4 source code that indicates it is possible to add multiple actions per type. However, this is not the case.

@benjaminhuth benjaminhuth added the Component - Examples Affects the Examples module label Apr 22, 2023
@benjaminhuth benjaminhuth added this to the next milestone Apr 22, 2023
@codecov
Copy link

codecov bot commented Apr 22, 2023

Codecov Report

Merging #2057 (7cda786) into main (2a90742) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2057   +/-   ##
=======================================
  Coverage   49.37%   49.37%           
=======================================
  Files         427      427           
  Lines       24779    24779           
  Branches    11430    11430           
=======================================
  Hits        12234    12234           
  Misses       4484     4484           
  Partials     8061     8061           

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

@asalzburger
Copy link
Contributor

Makes totally sense, thanks, @benjaminhuth

@asalzburger
Copy link
Contributor

Out Geant4 algorithm allows to pass std::vectors of user actions. However, multiple actions per type seem not to be supported by Geant4. This changes this to allow only one user action per type (run, event, particle, step).

I'm not sure if this was possible in former Geant4 versions, there is also a comment in the Geant4 source code that indicates it is possible to add multiple actions per type. However, this is not the case.

As of your question, I think this comes from the singleton usage/desing of Geant4 in the past. But as of the chances you did, they are all in line what Geant4 expects to do - and were just missing on our side.

@asalzburger
Copy link
Contributor

Out Geant4 algorithm allows to pass std::vectors of user actions. However, multiple actions per type seem not to be supported by Geant4. This changes this to allow only one user action per type (run, event, particle, step).
I'm not sure if this was possible in former Geant4 versions, there is also a comment in the Geant4 source code that indicates it is possible to add multiple actions per type. However, this is not the case.

As of your question, I think this comes from the singleton usage/desing of Geant4 in the past. But as of the chances you did, they are all in line what Geant4 expects to do - and were just missing on our side.

Oh - I misread this - I will check, I was under the impression that you can attach different UserActions, just not the the same one twice, I will check that.

@github-actions
Copy link

github-actions bot commented Apr 24, 2023

📊 Physics performance monitoring for 7cda786

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

@benjaminhuth benjaminhuth changed the title fix: broken Geant4 user actions configuration feature: particle killing 4 Geant4 and refactor user actions configuration Apr 24, 2023
@benjaminhuth benjaminhuth changed the title feature: particle killing 4 Geant4 and refactor user actions configuration feat: particle killing 4 Geant4 and refactor user actions Apr 24, 2023
Copy link
Contributor

@andiwand andiwand left a comment

Choose a reason for hiding this comment

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

nice catch - well done! left a few questions but looks good in general to me

@benjaminhuth
Copy link
Member Author

benjaminhuth commented Apr 26, 2023

@asalzburger I implemented the particle killing now with Acts::Volume, it feels much better now actually. Regarding other actions, I would like to wait until we need a similar list implementation if its ok for you. Is it good to go now from your perspective? (will need to update hashes, so formal approval is not yet required)

I also directly included the particle killing into full_chain_odd.py, so I also tag @andiwand as well. Is this okay or should I remove it again? I would like to execute this at least once in the CI.

@andiwand
Copy link
Contributor

I also directly included the particle killing into full_chain_odd.py, so I also tag @andiwand as well. Is this okay or should I remove it again? I would like to execute this at least once in the CI.

I wonder if we could have a more dedicated test for that. Or will the full chain actually break if you remove it?

I only worry that we might silently remove rouge particles which should not be there in the first place and we will catch the error at a different place later in time

@kodiakhq kodiakhq bot merged commit 9cff63a into acts-project:main Apr 27, 2023
38 checks passed
@benjaminhuth benjaminhuth deleted the refactor/geant4-configuration branch May 10, 2023 07:15
@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 - Examples Affects the Examples module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants