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 support for surface-based VecGeom #1412

Open
sethrj opened this issue Sep 18, 2024 · 7 comments
Open

Add support for surface-based VecGeom #1412

sethrj opened this issue Sep 18, 2024 · 7 comments
Assignees
Labels
external Dependencies and framework-oriented features

Comments

@sethrj
Copy link
Member

sethrj commented Sep 18, 2024

VecGeom is implementing a new surface-based navigation mechanism designed to be more efficient on GPU: see 10.2172/2204991 and associated AdePT integration apt-sim/AdePT#298 . This issue will track the progress of that integration.

@mrguilima please document the issues you're seeing as part of this update (upload log files of tests, etc.) Thanks!

@sethrj sethrj added the external Dependencies and framework-oriented features label Sep 18, 2024
@mrguilima
Copy link
Contributor

The first three failing tests need support for Orb (FourLevels*) and many other shapes (solids.gdml).
54 - geocel/vg/Vecgeom:FourLevelsGeantTest.* (SEGFAULT) <<== needs Orb
55 - geocel/vg/Vecgeom:SolidsGeantTest.* (SEGFAULT) <<== solids.gdml, needs support for ~all shapes
157 - celeritas/ext/GeantVolumeMapper (Failed) <<== also uses solids.gdml, needs support for ~all shapes

@sethrj
Copy link
Member Author

sethrj commented Sep 18, 2024

Thanks! Did you see Severin's comment about recent changes negating the need for the stack size? How recent is the build of vecgeom you're using?

Yes, as I said yesterday, the stack size issue appears only in Debug mode, not in Release mode.

For the missing "orb" we should be able to trivially build a sphere instead, and for the unsupported volumes (do you have a list?) we can likewise modify the geometry converter.

I can apply this change.

@mrguilima
Copy link
Contributor

mrguilima commented Sep 18, 2024

The next group of tests are failing because they load more than one geometry, but the reset_geometry() function is not working properly for the Surface infrastructure. Using the --gtest_filter=Cmse* etc.. works for most of the geometries though (exception: Cmse), so I assume that the BVHNavigator from VecGeom is working fine at this point.

162 - celeritas/field/FieldPropagator (SEGFAULT)
165 - celeritas/geo/Geometry (SEGFAULT)

All these work well:
test/celeritas/field_FieldPropagator --gtest_filter=TwoBox*
test/celeritas/field_FieldPropagator --gtest_filter=LayersTest*
test/celeritas/field_FieldPropagator --gtest_filter=SimpleCmsTest*
test/celeritas/field_FieldPropagator --gtest_filter=CmseTest*

test/celeritas/geo_Geometry --gtest_filter=SimpleCmsTest*
test/celeritas/geo_Geometry --gtest_filter=ThreeSpheres*

and this one fails:
test/celeritas/geo_Geometry --gtest_filter=Cmse*

Cmse geometry needs some attention.

@sethrj
Copy link
Member Author

sethrj commented Sep 18, 2024

Can you run ctest with --output-on-failure and print the within-test failures?

@mrguilima
Copy link
Contributor

This is the typical test output when a second geometry is loaded:

    Start 53: geocel/vg/Vecgeom:SimpleCmsTest.*
1/4 Test #53: geocel/vg/Vecgeom:SimpleCmsTest.* .........   Passed    0.94 sec
    Start 54: geocel/vg/Vecgeom:FourLevelsGeantTest.*
2/4 Test #54: geocel/vg/Vecgeom:FourLevelsGeantTest.* ...***Exception: SegFault  0.46 sec
Celeritas version 0.5.0-dev.225+0f952bc0
Note: Google Test filter = FourLevelsGeantTest.*
[==========] Running 2 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 2 tests from FourLevelsGeantTest
[ RUN      ] FourLevelsGeantTest.accessors
/global/homes/l/lima/work/local/src/vecgeom/repo/VecGeom/surfaces/conv/SolidConverter.h:95: error: CreateSolidSurfaces: solid type not supported UnplacedOrb {5}
/global/homes/l/lima/work/local/src/vecgeom/repo/VecGeom/surfaces/cpp/BrepHelper.cpp:1290: critical: Could not convert volume 0: Shape20x14a1d50
Table size index: 3032
unknown file: Failure
C++ exception with description "celeritas: runtime error: failed to convert VecGeom to surfaces
/global/homes/l/lima/work/cele/src/geocel/vg/VecgeomParams.cc:375: 'brep_helper.Convert()' failed" thrown in the test body.

[  FAILED  ] FourLevelsGeantTest.accessors (39 ms)

@mrguilima
Copy link
Contributor

And here is the output for Geometry:CmseTest*

 ==>>> test/celeritas/geo_Geometry --gtest_filter=CmseTest*
Celeritas version 0.5.0-dev.225+0f952bc0
Note: Google Test filter = CmseTest*
[==========] Running 2 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 2 tests from CmseTest
[ RUN      ] CmseTest.host
status: Loading VecGeom geometry from GDML at /global/homes/l/lima/work/cele/test/geocel/data/cmse.gdml
Table size index: 4080
status: Initializing tracking information
/global/homes/l/lima/work/cele/test/celeritas/geo/HeuristicGeoTestBase.cc:74: Failure
Values in: avg_path
 Expected: ref_path
14 of 18 elements differ
by 0.0050000000000000001 relative error or 5.0000000000000002e-05 absolute error
 i            ref_path           avg_path         Difference
 0            74.17136    74.6817891125235 0.00688175479758624
 1            13.25306    13.4613848262866  0.0157189982001622
 2            76.67924    67.7890370814263  -0.115940154317827
 3            449.5464    460.345985002743  0.0240232932634832
 4          0.09551618  0.0752032526740877  -0.212664779160057
 5           0.3231404   0.395826227083629   0.224935746454573
 6            0.310899   0.258379633374872   -0.16892742216967
 7           0.3844357    0.51484801200587   0.339230492916942
 9            11.09485    10.6629583649317 -0.0389272171384311
10            9.101073     9.3044714865428  0.0223488468384776
12           0.3033329   0.258743528862538  -0.146998136824137
14            228.7892    226.528020448794 -0.0098832442755416
16            563.0746    550.756536464024 -0.0218764326005393
17            2858.592    2824.10663159091 -0.0120637602040073


[  FAILED  ] CmseTest.host (210 ms)
[ RUN      ] CmseTest.device
/global/homes/l/lima/work/cele/test/celeritas/geo/HeuristicGeoTestBase.cc:98: Failure
Values in: avg_path
 Expected: this->reference_avg_path()
14 of 18 elements differ
by 0.0050000000000000001 relative error or 5.0000000000000002e-05 absolute error
 i            EXPECTED           avg_path         Difference
 0            74.17136    74.6817891125236 0.00688175479758797
 1            13.25306    13.4613848262866  0.0157189982001608
 2            76.67924    67.7890370814264  -0.115940154317826
 3            449.5464    460.345985002744  0.0240232932634845
 4          0.09551618  0.0752032526740867  -0.212664779160068
 5           0.3231404   0.395826227083628    0.22493574645457
 6            0.310899   0.258379633374872   -0.16892742216967
 7           0.3844357    0.51484801200587   0.339230492916943
 9            11.09485    10.6629583649317 -0.0389272171384296
10            9.101073    9.30447148654281  0.0223488468384788
12           0.3033329   0.258743528869017  -0.146998136802776
14            228.7892    226.528020448788 -0.00988324427556943
16            563.0746    550.756536464034 -0.0218764326005231
17            2858.592    2824.10663159092 -0.0120637602040014

[  FAILED  ] CmseTest.device (99 ms)
[----------] 2 tests from CmseTest (310 ms total)

@sethrj
Copy link
Member Author

sethrj commented Sep 19, 2024

Awesome, thanks @mrguilima . If you open a draft pull request, I can push patches to fix the "unplaced orb" issue and we can work from there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external Dependencies and framework-oriented features
Projects
None yet
Development

No branches or pull requests

2 participants