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.

Sign up for GitHub

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

Jump to bottom

Add new vecgeom tests to emulate CMS run 3 issues #800

Merged
merged 24 commits into from
Jun 9, 2023
Merged

Add new vecgeom tests to emulate CMS run 3 issues #800

merged 24 commits into from
Jun 9, 2023

Conversation

sethrj
Copy link
Member

@sethrj sethrj commented Jun 5, 2023 •
edited
Loading

  • Add a test to use the GeantImporter to read volume names to try to reproduce the missing _refl material IDs in CMS run 3
  • Add runtime validation to VecGeom GPU parameter construction
  • Skip volume checking in geo converter unless 'CELER_COMPARE_VOLUMES' is set, because this is slow for CMS run 3
  • Improve logging messages through ScopedRedirect moved to Improve logger interaction with Geant4 #804
  • Test behavior of tracking through reflected solids whose underlying solids/LV are not instantiated in the geometry
  • Reproduce the "There exists more than ONE logical volume in store" message seen in CMS run 3

@sethrj sethrj added enhancement New feature or request external Dependencies and framework-oriented features labels Jun 5, 2023
@sethrj sethrj requested review from pcanal and amandalund June 5, 2023 13:58
@sethrj sethrj mentioned this pull request Jun 5, 2023
Merged
5 tasks
@sethrj sethrj force-pushed the vecgeom-testing branch from 718f3aa to 7d90da0 Compare June 5, 2023 14:52
@sethrj sethrj requested review from mrguilima and removed request for pcanal June 5, 2023 18:50
@sethrj sethrj added documentation Documentation, examples, tests, and CI and removed enhancement New feature or request labels Jun 8, 2023
@sethrj sethrj changed the title Improve VecGeom testing and GDML importing Add new vecgeom tests to emulate CMS run 3 issues Jun 8, 2023
amandalund
amandalund approved these changes Jun 8, 2023
View reviewed changes
Copy link
Contributor

@amandalund amandalund left a comment

Choose a reason for hiding this comment

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

This looks good to me @sethrj, thanks for adding these!

test/celeritas/data/solids.gdml
<userinfo>
<auxiliary auxtype="Region" auxvalue="Turds">
<auxiliary auxtype="volume" auxvalue="trd1"/>
<auxiliary auxtype="volume" auxvalue="trd2"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

So mature seth ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

The error message is even better because it adds an exclamation point:

error: G4GDMLParser::ImportRegions():
celeritas: Geant4 error: Notification failed:
    There exists more than ONE logical volume in store named: trd3_refl.
NOTE: assigning all such volumes as root logical volumes for region: Turds!

@sethrj sethrj merged commit ec66b6f into celeritas-project:develop Jun 9, 2023
@sethrj sethrj deleted the vecgeom-testing branch June 9, 2023 02:10
@sethrj sethrj added documentation Documentation, examples, tests, and CI and removed documentation Documentation, examples, tests, and CI labels Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amandalund amandalund amandalund approved these changes

@mrguilima mrguilima Awaiting requested review from mrguilima

Assignees
No one assigned
Labels
documentation Documentation, examples, tests, and CI external Dependencies and framework-oriented features
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sethrj @amandalund