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

Add new capabilities to run CMS with sensitive detectors #713

Merged
merged 17 commits into from
Apr 10, 2023

Conversation

sethrj
Copy link
Member

@sethrj sethrj commented Apr 4, 2023

  • If multiple logical volumes share the same SD name, create a single SD to share among them
  • Add the ability to set the number of streams manually
  • Add an option to strip GDML pointers from the input file for the demo-geant app
  • Fix a crash when creating the hit manager when multiple volumes have the same name
  • Be smarter about appending uniquifying pointers when exporting to VecGeom

@whokion @amandalund With the new /setup/stripGDMLPointers true option, I can now successfully run CMS run 3 through the standalone app: see https://github.com/celeritas-project/benchmarks/blob/main/full-cms-test/demo-geant-cpu.mac .

There are still a bunch of warning messages such as:

VecgeomParams.cc:290: warning: Geometry contains duplicate volume names: "CTPPS_210_Left_Station_RP_210_Left_Station_Hor_Vacuum@0x60000051c160" (1603), "CTPPS_210_Left_Station_RP_210_Left_Station_Hor_Vacuum@0x60000051c160" (1601), "CTPPS_210_Left_Station_RP_210_Left_Station_Hor_Vacuum@0x60000051c160" (1517),
...
GeoMaterialParams.cc:154: warning: Some geometry volumes do not have known material IDs: pixfwdInnerDiskZplus_PixelForwardInnerDiskOuterRing_seg_1@0x11fe571c0, pixfwdInnerDiskZplus_PixelForwardInnerDiskOuterRing_seg_2@0x11fe572f0,
...

but these names are only inside of "union" solids, and I think they're just an artifact of how VecGeom builds temporary volumes that seem not to show up in the actual geometry.

@sethrj sethrj added documentation Documentation, examples, tests, and CI enhancement New feature or request labels Apr 4, 2023
@sethrj sethrj requested a review from whokion April 4, 2023 00:33
@sethrj sethrj requested a review from amandalund April 4, 2023 19:04
@sethrj sethrj added this to the v0.3.0 milestone Apr 4, 2023
@sethrj
Copy link
Member Author

sethrj commented Apr 4, 2023

The last round of fixes will conflict with #557 .

@whokion
Copy link
Contributor

whokion commented Apr 4, 2023

@sethrj I have a compilation error with the latest update:

celeritas/test/orange/OrangeGeoTestBase.hh:117:23: error: invalid use of incomplete type ‘class celeritas::OrangeParams’
  117 |         return params_->num_volumes();

@sethrj
Copy link
Member Author

sethrj commented Apr 4, 2023

Sorry @whokion , that's what I get for only building one test and the app. Everything now builds on my laptop, hope that fixes it!

@whokion
Copy link
Contributor

whokion commented Apr 4, 2023

Just for records, there are still two known issues running demo-geant-integration on GPU (even though this MR may not be directly responsible for these failures):

  1. failure from demo-geant-integration simple-cms-test.mac (with the default, maxNumTracks=2048) - runs okay, if maxNumTracks=1024
celeritas/src/celeritas/em/interactor/LivermorePEInteractor.hh:135:
celeritas: internal assertion failed: inc_energy_ > 0
  1. failure with cms-run3.gdml
      issued by : celeritas/ext/VecgeomParams.cc:213
internal assertion failed: world_top_devptr != nullptr

@amandalund
Copy link
Contributor

amandalund commented Apr 4, 2023

I am also still hitting internal assertion failed: world_top_devptr != nullptr with cms-run3 -- @sethrj it sounds like you are not (or are running on the CPU)?

And probably not related to this PR, but the following tests are now failing for me (vecgeom, debug on, reldeb build):

The following tests FAILED:
	 90 - celeritas/global/Stepper:TestEm3NoMsc.* (Failed)
	142 - app/demo-loop (Failed)
	144 - app/demo-geant-integration (Subprocess aborted)

@sethrj
Copy link
Member Author

sethrj commented Apr 5, 2023

@amandalund Yes, this was with CPU just on my laptop.

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.

Thanks for the fixes and improvements @sethrj!

@sethrj sethrj merged commit 2887001 into celeritas-project:develop Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation, examples, tests, and CI enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants