Remove hidden state from VecGeom
Chained on #2114 , #2115
As part of #2111 I wanted to eliminate the it-could-almost-be-useful fLastExited path that lives inside the VecGeom 2 NavState but is inaccessible from VG1 and not present in the NavPath implementation. We need something like it to fix #1921 because the normal may live on the parent or the child (before or after). So to avoid keeping multiple exiting/entering states, since we already have "next" and "current", I added some wrappers that look like a navstate but operate on references to the state/boundary values. (Storage is also easier and more efficient with these since boundary bools live side by side.)
Lo and behold, several of the vecgeom tests no longer needed the exception "is solid && vecgeom 2" added in #1720. The only difference is that we discard fLastExited after every VecGeom navigation call. This indicates the new logic for excluding volumes previously entered/exited is interfering with how we do tracking.
The next step after this will be merging the solid BVH navigators.
Implementation notes
CELER_VGNAVis defined based on vecgeom configuration toCELER_VGNAV_PATH,CELER_VGNAV_TUPLE, orCELER_VGNAV_INDEX. The "path" selection is unfortunately deployed on all vg1 CPU and has special treatment for its state allocation.- Vecgeom surface navigation does not need the 'next boundary' flag since it already has 'next surface'. We use that instead, but for the moment to keep the interface consistent still allocate a 'next boundary'.
VgNavStateWrapperhas the interface of a VG nav state, but it lacks the "last exited" state, and it simply forwards calls to the stored references to state/boundary. This lets us use it to do things like Push/Pop- For compatibility with other vecgeom functions that require the whole big state, there's a
ScopedVgNavStatethat allows the full states to be saved on the stack and reconstruct the pointed-to data on destruction (discarding fLastExited in the process). - Use Collection instead of
VecgeomNavCollectionfor all types except for the navigation path.
Codecov Report
:x: Patch coverage is 89.36170% with 5 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 84.89%. Comparing base (e5b85e6) to head (9c48bef).
:warning: Report is 4 commits behind head on develop.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/geocel/vg/detail/VecgeomNavCollection.hh | 58.33% | 5 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## develop #2116 +/- ##
===========================================
+ Coverage 84.86% 84.89% +0.02%
===========================================
Files 1253 1253
Lines 44135 44134 -1
Branches 16494 16646 +152
===========================================
+ Hits 37456 37468 +12
- Misses 4714 4875 +161
+ Partials 1965 1791 -174
| Files with missing lines | Coverage Δ | |
|---|---|---|
| src/corecel/OpaqueId.hh | 89.83% <ø> (ø) |
|
| src/geocel/vg/VecgeomData.hh | 87.75% <100.00%> (+0.57%) |
:arrow_up: |
| src/geocel/vg/VecgeomParams.cc | 65.44% <100.00%> (ø) |
|
| src/geocel/vg/VecgeomParams.hh | 84.61% <ø> (ø) |
|
| src/geocel/vg/VecgeomTrackView.hh | 82.41% <100.00%> (+0.91%) |
:arrow_up: |
| src/geocel/vg/detail/BVHNavigator.hh | 83.47% <100.00%> (-0.70%) |
:arrow_down: |
| src/geocel/vg/detail/VecgeomNavCollection.cc | 100.00% <100.00%> (+56.25%) |
:arrow_up: |
| src/geocel/vg/detail/VecgeomNavCollection.hh | 57.14% <58.33%> (-19.79%) |
:arrow_down: |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Test summary
5 702 files 9 168 suites 18m 2s :stopwatch: 2 080 tests 2 054 :white_check_mark: 26 :zzz: 0 :x: 31 837 runs 31 689 :white_check_mark: 148 :zzz: 0 :x:
Results for commit 9c48befd.
:recycle: This comment has been updated with latest results.
Regarding the NavIndexPath option for CPU, since it is not compatible with GPU, it gets automatically "upgraded" out of the *Path option when GPU is enabled.
@mrguilima This is ready for review.
Ugh, bad news: as of this commit, vecgeom 2 fails
with config:
{"_category":"system","_label":"build","config":{"build_type":"relwithdebinfo,shared","core_geo":"VecGeom","core_rng":"xorwow","debug":true,"geant4":"global-dynamic,multithreaded,shared,gdml","gpu_architectures":"90","hostname":"hudson","openmp":"disabled","real_type":"double","units":"CGS","use":["covfie","cuda","geant4","hepmc3","root","vecgeom"],"vecgeom":"bvh_single,tuple,Scalar","versions":{"CLHEP":"2.4.7.1","CUDA":"12.9.86","G4VG":"1.0.5","Geant4":"11.3.2","HepMC3":"3.03.01","ROOT":"6.36.04","Thrust":"","VecGeom":"2.0.0-dev.7","covfie":"0.15.3"}},"version":"0.7.0-dev.204+71736034a"}
before:
The following tests FAILED:
232 - celeritas/geo/Geometry:SimpleCms* (Failed) gpu unit
234 - celeritas/geo/Geometry:ThreeSpheres* (Failed) gpu unit
after (with a patch to fix missing vecgeom IWYU):
The following tests FAILED:
87 - geocel/vg/Vecgeom:SimpleCmsVgdmlTest.* (Subprocess aborted) gpu unit
232 - celeritas/geo/Geometry:SimpleCms* (Failed) gpu unit
234 - celeritas/geo/Geometry:ThreeSpheres* (Failed) gpu unit
317 - celeritas/track/TrackSort (Failed) gpu unit
323 - celeritas/user/SlotDiagnostic (Failed) gpu unit
354 - accel/TrackingManagerIntegration:LarSphere.run:gpu:mt (Failed) gpu unit
355 - accel/TrackingManagerIntegration:LarSphere.run:gpu:task (Failed) gpu unit
Looks like this is because of my hacky (incorrect+illegal) use of NavStateTuple as POD.