test(geom): add dRICH geometry tests/geom/drich.gdml - #427
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends Simphony’s CSG leaf representation and intersection/SDF logic to support partial-phi (“phi wedge”) intervals baked directly into centered CSG_ZSPHERE and CSG_CYLINDER leaves, and adds regression tests/fixtures to validate both conversion and intersection behavior.
Changes:
- Add
startPhi/deltaPhi-awaresn::ZSphereandsn::Cylinderleaf constructors and update AABB handling accordingly. - Update U4 solid conversion to preserve partial-phi for outer sphere/tube shells while avoiding coincident radial walls in boolean shells.
- Add new unit tests (U4 + CSG) and a GDML fixture to validate conversion outputs and radial-wall intersection ordering.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| u4/U4Solid.h | Preserves partial-phi on outer sphere/tube leaves; adds boolean-side phi-wedge de-dup to avoid coincident radial walls. |
| sysrap/sn.h | Adds phi-wedge overload constructors for Cylinder/ZSphere and updates leaf-frame AABB logic for the new parameter layout. |
| CSG/csg_intersect_leaf_phi_wedge.h | Introduces shared helpers for phi-wedge inclusion tests, signed field, and radial-wall intersections. |
| CSG/csg_intersect_leaf_zsphere.h | Updates ZSphere distance/intersection to apply phi-wedge constraints and support radial-wall hits. |
| CSG/csg_intersect_leaf_cylinder.h | Updates Cylinder distance/intersection to apply phi-wedge constraints and support radial-wall hits. |
| CSG/CSGNode.cc | Updates cylinder local AABB computation consistent with the cylinder leaf’s parameter layout. |
| CSG/csg_robust_quadratic_roots.h | Removes inactive preprocessor branches, leaving the robust quadratic-root path. |
| CSG/csg_intersect_leaf_sphere.h | Removes inactive preprocessor branch, always using robust quadratic roots. |
| CSG/tests/intersect_leaf_phi_wedge_test.cc | Adds targeted test coverage for curved-surface, cap, and radial-wall behavior for phi-wedged leaves. |
| CSG/tests/CMakeLists.txt | Registers the new intersect_leaf_phi_wedge_test. |
| CSG/CMakeLists.txt | Registers the new phi-wedge intersect header for build/install. |
| u4/tests/SpherePhiCutQuarterShellTest.cc | Strengthens sphere conversion test to assert canonical “wedged outer + full inner” behavior for GDML and native shells. |
| u4/tests/TubePhiCutQuarterShellTest.cc | Adds tube conversion test asserting canonical “wedged outer + full inner cylinder” behavior for a quarter-phi shell. |
| u4/tests/CMakeLists.txt | Registers the new tube quarter-shell test. |
| tests/geom/tube_phicut_quarter_shell.gdml | Adds GDML fixture defining a quarter-phi annular tube used by the new U4 test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7729e6d771
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The gdml file is from this commit 1c8bb29
The geoConverter-exported dRICH fixture from PR #347 defines optical sensor skin surfaces but does not identify their logical volumes as Geant4 sensitive detectors. simg4ox constructs PhotonSD instances from the GDML auxiliary map, so photons reaching these surfaces are otherwise absent from the Geant4 hit collection and cannot be compared with Opticks GPU hits. Add a SensDet=PhotonDetector auxiliary record to each of the 210 DRICH_pss logical volumes. This preserves the original solids, placements, materials, and optical surfaces while allowing the CPU simulation to attach PhotonSD instances and record the sensor hits needed by the dRICH integration comparison.
The dRICH GDML exported by DD4hep stores optical-property energy axes in GeV, while Geant4 imports the raw matrix coordinates using its MeV-based internal units. This leaves material and surface spectra 1000 times below their intended energies, moving them outside the optical-photon range and suppressing physical transport and detection. Scale the energy coordinate of all 41 two-column property tables by 1000 while preserving their property values. Also model the photon-sensor surface as unified dielectric-metal with explicit zero reflectivity so Geant4 enters the EFFICIENCY absorption/detection path rather than applying dielectric Fresnel transport or reflecting photons before detection. These corrections encode the import-time adjustments from the PR #347 standalone dRICH driver directly in the reusable fixture, enabling consistent Geant4 CPU and Opticks GPU hit comparisons while retaining all 210 sensitive-detector annotations.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Add the dRICH GDML geometry fixture from #347 for reusable Geant4–Opticks testing.