Skip to content

Skip boundary searches inside a safety sphere during field propagation - #2578

Closed
esseivaju wants to merge 5 commits into
celeritas-project:developfrom
esseivaju:field-safety-sphere
Closed

esseivaju wants to merge 5 commits into
celeritas-project:developfrom
esseivaju:field-safety-sphere

Conversation

@esseivaju

Copy link
Copy Markdown
Member

Charged-particle propagation in a magnetic field dominates GPU time in the ATLAS (Athena) offload. In a 4-event single-thread profile on an A100 (VecGeom, CartMapField), the along-step-cartmap-msc-propagate kernel takes 51% of all kernel time. More than 90% of the total is geometry navigation plus the field integration that surrounds it.

FieldPropagator searched for a boundary along the chord of every substep, even deep inside a volume. This PR keeps a lazily computed safety sphere: a ball centered on an earlier substep position, with the safety distance there as its radius, so it contains no boundary. A substep is accepted without a boundary search when its chord end point, plus the intersection tolerance, lies inside the sphere. That search would have found no boundary, so trajectories are unchanged.

The sphere is computed lazily so its cost stays bounded:

  • It is never computed before the first internal substep, so a propagation that takes a single substep does no extra work.
  • It is not computed for the substep that completes the step, because that would at best trade one boundary search for one safety query.
  • After a recomputed sphere fails to contain the chord, it is not recomputed again in the same call, so at most one safety query per call is wasted.

Changes

  • detail::SafetySphere (field/detail/FieldUtils.hh): a helper that tests whether a point, with a tolerance, is inside the sphere.
  • FieldPropagator: the safety-sphere skip described above. The substep acceptance logic now lives in a local helper shared by both paths, and the algorithm is documented in the class docs.
  • find_safety(max) respects its maximum in ORANGE and Geant4:
    • The interface says the result should not exceed the requested maximum, and VecGeom already truncates it.
    • ORANGE ignored the argument, and the Geant4 wrapper returned its cached safety.
    • Both are now truncated, which resolves the TODO in GeoTests.cc and the related CheckedGeoTrackView warnings.
    • UrbanMsc only compares the safety against the maximum it requests, so MSC results are unchanged.
  • CheckedGeoTrackView no longer warns about a zero safety when the geometry reports that it doesn't support safety. ORANGE legitimately returns 0 for surfaces without a simple safety.

Results

The CmseTest.coarse field propagation test runs 4 track radii. Boundary crossings, steps, and integrations are identical to before for both VecGeom and ORANGE. Only the navigation call counts change:

Geometry Boundary searches before Boundary searches after Safety queries added
VecGeom 30419 / 19551 / 16170 / 9956 11176 / 7120 / 3634 / 4863 9954 / 6424 / 3285 / 1944
ORANGE 30419 / — / 16170 / 9956 12548 / — / 6128 / 5925 9954 / — / 3268 / 1835

The second radius is the known ORANGE failure case. The total number of navigation queries drops by 30–57%. A test that crosses many thin layers with short steps (LayersTest.revolutions_through_layers) performs no safety queries at all.

Testing

  • New and updated tests:
    • SafetySphereTest.contains;
    • navigation-count checks in TwoBoxesTest.electron_interior and LayersTest.revolutions_through_layers;
    • a num_safety column in the CMSE regression result.
  • The CMSE test checks two kinds of results:
    • Trajectory results (boundary crossings, steps, integrations): compared exactly for every geometry.
    • Navigation counts (boundary searches, safety queries): these depend on each geometry's safety distances.
      • ORANGE and VecGeom 2.x, where they were measured: compared exactly.
      • Other geometries (VecGeom 1.x, Geant4 core geometry): checked against two bounds instead. The run must skip some boundary searches, and it may waste at most one safety query per propagation.

Follow-up

Seed the initial sphere from the safety distance that UrbanMsc::limit_step already computes at the start of each step. The first substep could then also skip the boundary search. This needs per-track storage of the safety and its center, and is noted as a \todo in the FieldPropagator docs.

Assisted-by: Claude Code (Claude Opus 5.5)

Add a detail::SafetySphere helper that tests whether a point, with a
tolerance, is inside a boundary-free sphere. Move the bookkeeping for
accepting an internal substep in FieldPropagator into a local lambda so
that a following change can reuse it. This does not change behavior.

Assisted-by: Claude Code (Claude Opus 5.5)
The geometry interface documents that find_safety(max_step) returns no
more than the maximum, and VecGeom already truncates its result. ORANGE
ignored the argument and the Geant4 wrapper returned its cached safety,
so both could return larger values. Truncate the result in both, which
resolves the TODO in the safety geometry test and removes the
corresponding CheckedGeoTrackView warnings.

Callers in UrbanMsc only compare the safety against the requested
maximum, so their results are unchanged.

Assisted-by: Claude Code (Claude Opus 5.5)
A geometry that does not support safety calculations, such as ORANGE
with surfaces lacking a simple safety, legitimately returns zero safety.
Only warn about a zero safety in CheckedGeoTrackView if the geometry
reports that it supports safety, as the internal-move safety check
already does.

Assisted-by: Claude Code (Claude Opus 5.5)
Every field propagation substep searched for a boundary along its chord,
even far from any boundary. Keep a lazily calculated safety sphere in
FieldPropagator and accept a substep without the search when the chord
end point, plus the intersection tolerance, lies inside it. The search
would have found no boundary, so trajectories are unchanged.

The sphere is calculated only after an internal move and when more
substeps may follow, and is not recalculated again in the same call once
a new sphere fails to contain the chord, so at most one safety
calculation per propagation is wasted.

In the CMSE field test, the boundary, step, and integration counts are
unchanged while intersection searches drop from {30419, 19551, 16170,
9956} to {11176, 7120, 3634, 4863} with VecGeom, at the cost of {9954,
6424, 3285, 1944} safety calculations. A test that crosses many thin
layers with short steps performs no safety calculations.

Assisted-by: Claude Code (Claude Opus 5.5)
The number of boundary searches skipped by the safety sphere depends on
the safety distances that each geometry calculates, which differ between
ORANGE, VecGeom 1.x, VecGeom 2.x, and Geant4. Only compare the CMSE
navigation counts exactly for ORANGE and VecGeom 2.x, where they were
measured. Other configurations still check the trajectory results
exactly, and check that some boundary searches are skipped and that
safety calculations stay within one wasted call per propagation.

Assisted-by: Claude Code (Claude Opus 5.5)
@esseivaju esseivaju added ai-assisted Generated/refactored substantially with agentic/LLM AI tools field Magnetic field and propagation performance Changes for performance optimization labels Oct 22, 2026
@github-actions

Copy link
Copy Markdown

Test summary

 6 066 files  10 057 suites   6m 48s ⏱️
 1 945 tests  1 932 ✅  13 💤 0 ❌
34 047 runs  33 857 ✅ 190 💤 0 ❌

Results for commit 8c8e965.

@sethrj

sethrj commented Oct 22, 2026

Copy link
Copy Markdown
Member

@esseivaju Let's hold off on this before we have a strategy to improve and test especially for the computationally intensive use cases. I don't think safety sphere will work with orange, and as I said earlier, I'm not convinced it's the right approach for calorimeters and trackers (which all have very thin slices).

Before we add more complexity to this class I'd like the AI to refactor the field propagator and some of the boundary mechanics so we can improve testing and extensibility.

Not sure if your AI saw these:

@sethrj

sethrj commented Oct 22, 2026

Copy link
Copy Markdown
Member

Also I think this refactor should be done in conjunction with MSC reconsiderations, and the MSC also definitely needs refactoring.

But if you just want to try some stuff in this branch and see if you get a performance boost for athena, please by all means go for it :)

@esseivaju

Copy link
Copy Markdown
Member Author

Yeah I mainly wanted to test whether this was helping in Athena, though as you say within complex geometry l it doesn't really help. This gives a maybe 2-3% improvement in Athena... Interestingly a lot of the kernel time seems to be caused by a few tracks: only 13% of the steps have a propagation kernels taking more than 8ms, yet those 13% of steps represent 47% of the propagation time.

@esseivaju esseivaju closed this Oct 23, 2026
@sethrj

sethrj commented Oct 23, 2026

Copy link
Copy Markdown
Member

@esseivaju There's definitely a lot of low-hanging fruit for the field propagator optimization. Better characterizing the behavior in a realistic simulation would be a great first step though: maybe some counters for histograms of tracks -> steps -> substeps -> field convergence loops -> boundary tests ? Easy with AI, super valuable in practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-assisted Generated/refactored substantially with agentic/LLM AI tools field Magnetic field and propagation performance Changes for performance optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants