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: Geant4SurfaceProvider optimization #3025

Merged
merged 8 commits into from
Mar 20, 2024

Conversation

ssdetlab
Copy link
Contributor

Introducing minor changes to the Geant4SurfaceProvider targeting optimization and utility for Telescope-style use cases.

The first change is the removal of the Gdml parsing from the construction in favor of supplying a pointer to the world volume. The Internal Structure Builder is called (potentially) multiple times in the builder pipeline, so, before the changes, the description was also parsed multiple times and several Worlds were kept in the memory.

The second change has to do with the conventional z-orientation of the Telescope geometries. There's a need to rotate the geometries away from the z-axis in the current implementation of the track parametrization, so a simple way of applying a global transformation at the construction stage is introduced. In our experience, the cleanest way to do it is on the Geant4 volumes conversion stage, thus the additional G4Transform is included into the Config.

@github-actions github-actions bot added the Component - Plugins Affects one or more Plugins label Mar 12, 2024
@ssdetlab ssdetlab changed the title reafactor: Geant4SurfaceProvider optimization feat: Geant4SurfaceProvider optimization Mar 12, 2024
Copy link

codecov bot commented Mar 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.82%. Comparing base (e033607) to head (81d14b8).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3025   +/-   ##
=======================================
  Coverage   48.82%   48.82%           
=======================================
  Files         489      489           
  Lines       28860    28860           
  Branches    13695    13695           
=======================================
  Hits        14092    14092           
  Misses       4953     4953           
  Partials     9815     9815           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

andiwand
andiwand previously approved these changes Mar 18, 2024
@andiwand andiwand added this to the next milestone Mar 18, 2024
@andiwand
Copy link
Contributor

this does not compile anymore @ssdetlab. can you take a look?

@ssdetlab
Copy link
Contributor Author

As I mentioned in #3038, it was going to merge a test that is not compatible with the interface changes I propose here. Should be minor, so will fix soon

@ssdetlab ssdetlab requested a review from andiwand March 19, 2024 19:45
@kodiakhq kodiakhq bot merged commit c3eea1c into acts-project:main Mar 20, 2024
54 checks passed
@acts-project-service
Copy link
Collaborator

acts-project-service commented Mar 20, 2024

🔴 Athena integration test results [c3eea1c]

Build job with this PR failed!

Please investigate the build job for the pipeline!

@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Mar 20, 2024
@paulgessinger paulgessinger modified the milestones: next, v33.1.0 Mar 26, 2024
@andiwand andiwand removed the Breaks Athena build This PR breaks the Athena build label Apr 10, 2024
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 6, 2024
Introducing minor changes to the `Geant4SurfaceProvider` targeting optimization and utility for Telescope-style use cases. 

The first change is the removal of the Gdml parsing from the construction in favor of supplying a pointer to the world volume. The Internal Structure Builder is called (potentially) multiple times in the builder pipeline, so, before the changes, the description was also parsed multiple times and several Worlds were kept in the memory. 

The second change has to do with the conventional z-orientation of the Telescope geometries. There's a need to rotate the  geometries away from the z-axis in the current implementation of the track parametrization, so a simple way of applying a global transformation at the construction stage is introduced. In our experience, the cleanest way to do it is on the Geant4 volumes conversion stage, thus the additional `G4Transform` is included into the `Config`.
asalzburger pushed a commit to asalzburger/acts that referenced this pull request May 21, 2024
Introducing minor changes to the `Geant4SurfaceProvider` targeting optimization and utility for Telescope-style use cases. 

The first change is the removal of the Gdml parsing from the construction in favor of supplying a pointer to the world volume. The Internal Structure Builder is called (potentially) multiple times in the builder pipeline, so, before the changes, the description was also parsed multiple times and several Worlds were kept in the memory. 

The second change has to do with the conventional z-orientation of the Telescope geometries. There's a need to rotate the  geometries away from the z-axis in the current implementation of the track parametrization, so a simple way of applying a global transformation at the construction stage is introduced. In our experience, the cleanest way to do it is on the Geant4 volumes conversion stage, thus the additional `G4Transform` is included into the `Config`.
@ssdetlab ssdetlab deleted the g4-surface-provider-optimization branch October 11, 2024 12:41
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.

4 participants