fix(u4solid): remove sphere deltaPhi assertion that crashes on dRICH mirror#289
fix(u4solid): remove sphere deltaPhi assertion that crashes on dRICH mirror#289ggalgoczi wants to merge 2 commits into
Conversation
The assertion required deltaPhi == 2*pi for all spheres, but the dRICH mirror sphere has deltaPhi < 2*pi from DD4hep construction. The phi clipping is handled by the parent IntersectionSolid, so the sphere primitive does not need to enforce it.
plexoos
changed the title
|
I’m concerned this change is broader than the problem it is trying to solve. Deleting the assert does unblock the dRICH case, but it also removes the only fail-fast check that told us when a That may be acceptable for the specific dRICH mirror if the DD4hep construction is really I also looked at the old So I think we should either:
|
I agree, this could cause problems for geometries, that have spherical mirrors and are not clipped. I can not think of such geometry right now. However I agree that this should not be a final fix. I would choose solution number 1. However I am unsure how to detect "this is the dRICH construction" from inside init_Sphere. The converter only sees the G4Sphere primitive. What do you suggest? |
|
@plexoos what exactly is required from your side to merge this? Can you address my questions? |
Add `SpherePhiCutQuarterShellTest` and a minimal GDML fixture covering a partial-phi spherical shell with no parent `G4IntersectionSolid`. A proposed change in `U4Solid::init_Sphere_()` removes the assert guarding `deltaPhi != 2*pi` (#289). That allows conversion to proceed for phi-cut `G4Sphere` inputs even though `init_Sphere_()` still only produces `sn::Sphere` or `sn::ZSphere` and does not model the phi cut itself. In the general case this risks silently converting a clipped spherical shell into a full-phi shell instead of failing fast. This test records the current standalone partial-phi sphere case and keeps it usable across both implementation states. It loads a reduced GDML reproducer, verifies that the shape contains partial-phi sphere primitives and no parent `G4IntersectionSolid`, then runs `U4Solid::Convert()` in a child process so CTest can observe either the current fail-fast rejection or a future successful conversion without killing the test runner. Keeping this regression test separate from any geometry-specific intersection-clipping case makes the intent explicit: removing the assert in `U4Solid::init_Sphere_()` is not a safe general fix unless real sphere phi-cut support is added. If such support is added later, this test continues to cover the fixture and conversion path, while a separate positive test should verify the geometric correctness of the resulting phi-cut solid.
Add `SpherePhiCutQuarterShellTest` and a minimal GDML fixture covering a partial-phi spherical shell with no parent `G4IntersectionSolid`. A proposed change in `U4Solid::init_Sphere_()` removes the assert guarding `deltaPhi != 2*pi` (#289). That allows conversion to proceed for phi-cut `G4Sphere` inputs even though `init_Sphere_()` still only produces `sn::Sphere` or `sn::ZSphere` and does not model the phi cut itself. In the general case this risks silently converting a clipped spherical shell into a full-phi shell instead of failing fast. This test records the current standalone partial-phi sphere case and keeps it usable across both implementation states. It loads a reduced GDML reproducer, verifies that the shape contains partial-phi sphere primitives and no parent `G4IntersectionSolid`, then runs `U4Solid::Convert()` in a child process so CTest can observe either the current fail-fast rejection or a future successful conversion without killing the test runner. Keeping this regression test separate from any geometry-specific intersection-clipping case makes the intent explicit: removing the assert in `U4Solid::init_Sphere_()` is not a safe general fix unless real sphere phi-cut support is added. If such support is added later, this test continues to cover the fixture and conversion path, while a separate positive test should verify the geometric correctness of the resulting phi-cut solid.
Add `SpherePhiCutQuarterShellTest` and a minimal GDML fixture covering a partial-phi spherical shell with no parent `G4IntersectionSolid`. A proposed change in `U4Solid::init_Sphere_()` removes the assert guarding `deltaPhi != 2*pi` (#289). That allows conversion to proceed for phi-cut `G4Sphere` inputs even though `init_Sphere_()` still only produces `sn::Sphere` or `sn::ZSphere` and does not model the phi cut itself. In the general case this risks silently converting a clipped spherical shell into a full-phi shell instead of failing fast. This test records the current standalone partial-phi sphere case and keeps it usable across both implementation states. It loads a reduced GDML reproducer, verifies that the shape contains partial-phi sphere primitives and no parent `G4IntersectionSolid`, then runs `U4Solid::Convert()` in a child process so CTest can observe either the current fail-fast rejection or a future successful conversion without killing the test runner. Keeping this regression test separate from any geometry-specific intersection-clipping case makes the intent explicit: removing the assert in `U4Solid::init_Sphere_()` is not a safe general fix unless real sphere phi-cut support is added. If such support is added later, this test continues to cover the fixture and conversion path, while a separate positive test should verify the geometric correctness of the resulting phi-cut solid.
2 participants
The assertion required deltaPhi == 2pi for all spheres, but the dRICH mirror sphere has deltaPhi < 2pi from DD4hep construction. The phi clipping is handled by the parent IntersectionSolid, so the sphere primitive does not need to enforce it.