Pr/rzmap covfie backend#2344
Merged
sethrj merged 23 commits intoApr 13, 2026
Merged
Conversation
added 5 commits
March 30, 2026 22:24
Define CovfieRZFieldTraits for 2D (R,Z) field interpolation, mirroring the existing 3D CovfieFieldTraits. The host pipeline uses affine->clamp->linear->strided->array with float2 storage, while the CUDA device path uses hardware texture interpolation for performance. Prompt: "Replace RZMapField with covfie backend for storage and bilinear interpolation on the R-Z grid, with CUDA texture acceleration on GPU" Assisted-by: Claude Opus 4.6
Add RZMapFieldData.covfie.hh with 4 ownership specializations (host value/reference, device value/reference) using CovfieRZFieldTraits, and RZMapField.covfie.hh which evaluates the 2D (R,Z) field via covfie and projects Br to Cartesian Bx/By components. Prompt: "Add covfie backend for RZMapField: create data ownership types and field evaluation class mirroring the CartMapField covfie pattern" Assisted-by: Claude Opus 4.6
Add RZMapFieldParams.covfie.cc with pimpl-based constructor that builds a 2D covfie field from RZMapFieldInput, transposing from [Z][R] input stride to covfie's (R,Z) ordering. Update RZMapFieldParams.hh to use pimpl when covfie is enabled and ParamsDataStore when disabled. Prompt: "Add covfie params constructor for RZMapField, mirroring CartMapFieldParams.covfie.cc with pimpl pattern" Assisted-by: Claude Opus 4.6
Add #if CELERITAS_USE_COVFIE guards to RZMapFieldData.hh and RZMapField.hh to dispatch to covfie implementations when enabled, preserving the original UniformGrid+Collection code path when disabled. Guard RZMapFieldParams.cc with #if !CELERITAS_USE_COVFIE. Add RZMapFieldParams.covfie.cc to the covfie CMake source list. Prompt: "Wire the covfie dispatch into existing RZMapField headers and CMakeLists.txt, matching CartMapField pattern" Assisted-by: Claude Opus 4.6
Add a CUDA/HIP device test that evaluates RZMapField on GPU and compares against expected values from CUDA texture interpolation. Update the host test expected values to use #if CELERITAS_USE_COVFIE so both covfie (bilinear) and non-covfie (1D-per-component) code paths are tested. Prompt: "Add device test for RZMapField covfie backend and ensure host test works for both covfie-enabled and covfie-disabled builds" Assisted-by: Claude Opus 4.6
rahmans1
added
enhancement
Test summary 6 020 files 9 737 suites 19m 24s ⏱️ Results for commit 218cfa4. ♻️ This comment has been updated with latest results. |
Update the #else branch expected values to match current upstream develop, which changed after the cms-tiny.field.json test data was updated. Prompt: "Fix CI failure: non-covfie RZMapFieldTest.all expected values are stale and don't match current upstream develop" Assisted-by: Claude Opus 4.6
Replace cuda_texture with cuda_device_array as the device storage backend for the RZ field map, unifying the CUDA/HIP/CPU backend chain to use the same affine<clamp<linear<strided<>>>> pipeline with software interpolation on all platforms. Build the device field bottom-up using cross-type constructors so each layer preserves its configuration during the host-to-device transfer. Remove incorrect CELER_FUNCTION annotations from host-only methods in value<device>, initialize field_view pointer to nullptr, replace std::hypot with the device-safe hypot from corecel/math/Algorithms.hh, and fix the non-covfie stub Impl/ImplDeleter definitions to compile without covfie. Update device test expected values to match software interpolation (identical to host) and tighten tolerance to 1e-7. Prompt: "Switch RZMapField covfie CUDA backend from cuda_texture to cuda_device_array with bottom-up layer construction, fix host-only CELER_FUNCTION annotations, and update device test expected values" Assisted-by: Claude Sonnet 4.6
Contributor
Author
Test Failure AnalysisEnvironment: Observe following test failure: ResolutionHowever, the failure mostly goes away if the following patch is applied to covfie |
added 3 commits
April 8, 2026 20:40
Add RZMapFieldTest.interp_validation with two synthetic cases whose values at non-grid points are analytically known: - Case 1 (both paths): Br=r, Bz=z — linear functions reproduced exactly by both 1D-per-component and covfie bilinear interpolation - Case 2 (covfie only): Br=r+z, Bz=r+z — bilinear exactly reproduces linear functions with cross-axis dependence, confirming covfie does true 2D interpolation; guarded by CELERITAS_USE_COVFIE since 1D-per-component correctly does not reproduce cross-axis variation Prompt: "Add a test that validates RZMapField interpolation against analytically known values using synthetic linear fields: one that varies on a single axis (ground truth for both paths) and one that varies on both axes (proving covfie does true 2D bilinear interpolation)" Assisted-by: Claude Sonnet 4.6
Covfie stores field values as float32 internally; GPU arithmetic produces slightly different rounding from CPU for 3 of 24 sample points (~1.03e-7 relative error), just exceeding the original 1e-7 tolerance. Widening to 2e-7 accommodates float32 precision on device while remaining tight enough to catch significant regressions. Prompt: "Widen RZMapFieldTest.device tolerance from 1e-7 to 2e-7 to accommodate float32 GPU rounding differences introduced by covfie's internal float storage" Assisted-by: Claude Sonnet 4.6
- msc_rzfield: add #if CELERITAS_USE_COVFIE branch with covfie bilinear interpolation values (displacement=0.495, angle=-0.927); the large difference from the non-covfie values (displacement=0.552, angle=0.095) arises because the test particle is in a region of strong cross-axis field gradients where 1D-per-component and bilinear interpolation diverge - msc_rzfield_finegrid: widen angle tolerance from EXPECT_SOFT_EQ to EXPECT_SOFT_NEAR with 1e-11, accommodating a 6e-12 relative difference due to float32 field storage in covfie Prompt: "Update SimpleCmsRZFieldAlongStepTest to add covfie-specific expected values for msc_rzfield where bilinear interpolation gives physically different results from 1D-per-component in strong gradient regions, and widen the msc_rzfield_finegrid angle tolerance to accommodate float32 rounding from covfie's internal field storage" Assisted-by: Claude Sonnet 4.6
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #2344 +/- ##
===========================================
- Coverage 87.26% 87.19% -0.08%
===========================================
Files 1382 1385 +3
Lines 43943 43998 +55
Branches 13949 13479 -470
===========================================
+ Hits 38346 38363 +17
+ Misses 4523 4401 -122
- Partials 1074 1234 +160
🚀 New features to boost your workflow:
|
Replace 20-line bottom-up manual layer-by-layer construction with a single cross-type field copy: field = std::make_unique<field_t>(*other.field). covfie's cross-type field constructor propagates through all transformer layers (affine->clamp->linear->strided), with strided's cross-type owning_data_t constructor triggering cuda_device_array's constructor from array_1d_like backends, performing the H2D transfer at the bottom of the chain. Prompt: "use the cross-copy constructor approach to simplify H2D device field construction instead of manually building from the bottom up layer by layer" Assisted-by: Claude Sonnet 4.6
sethrj
reviewed
Apr 9, 2026
added 6 commits
April 9, 2026 10:27
The CI has a C++17/C++20 split: library sources compile as C++20 but
some test files (e.g. AlongStep.test.cc) compile as C++17. When
CELERITAS_USE_COVFIE is ON, the include chain:
AlongStepRZMapFieldMscAction.hh -> RZMapFieldData.hh
-> RZMapFieldData.covfie.hh -> detail/CovfieRZFieldTraits.hh
-> covfie/core/parameter_pack.hpp (uses C++20 "requires")
caused C++17 builds to fail.
Fix by ensuring covfie types never appear in any .hh included by C++17
TUs:
- RZMapFieldData.hh (covfie path): define only the primary template
declaration and the const_reference/device specialization with a
void const* field_view (no covfie types). The other three
specializations remain in RZMapFieldData.covfie.hh which is only
included from .covfie.cc and .cu files.
- RZMapFieldData.covfie.hh: include RZMapFieldData.hh for the primary
template; remove the const_reference/device specialization (now in
.hh).
- RZMapField.covfie.hh: add rzmap_get_view<M> function template that
uses if constexpr (with M as template parameter so the discarded
branch is not instantiated) to either cast void const* on device or
call get_view() on host. Store ParamsRef const& instead of a direct
field view reference.
- RZMapFieldParams.covfie.cc: assign device_ref_ fields explicitly
since const_reference/device no longer has an operator=(value/device).
- AlongStepRZMapFieldMscAction.hh: remove spurious include of
RZMapFieldData.hh (the class only holds SPConstFieldParams; it never
references RZMapFieldParamsData directly).
The non-covfie build path and the CartMapField backend are unaffected.
Prompt: "The CI has a C++17/C++20 split where library source files compile
as C++20 but some test files compile as C++17. When CELERITAS_USE_COVFIE
is ON, covfie's parameter_pack.hpp uses C++20 requires clauses that leak
into C++17 TUs through the public RZMapFieldData.hh include chain. Fix
this by hiding all covfie types from public headers so that C++17 TUs
never see covfie includes. Use void const* type erasure for the device
const_reference specialization, with the cast back to the concrete view
type happening only in .cu files compiled as C++20."
Assisted-by: Claude Sonnet 4.6
The action's .cc and .cu files include RZMapField.hh, which when covfie is enabled pulls in covfie headers requiring C++20. The file was previously registered via the unconditional celeritas_polysource() call, placing it in the main celeritas library target that does not directly link Extcovfie and therefore does not inherit cxx_std_20 from covfie's INTERFACE_COMPILE_FEATURES. Move the action into _celer_covfie_src alongside AlongStepCartMapFieldMscAction so it is compiled in a target that directly links Extcovfie. Add the corresponding non-covfie fallback in the else() branch and remove the unconditional celeritas_polysource() call. Prompt: "AlongStepRZMapFieldMscAction.cc includes RZMapField.hh which pulls in covfie headers requiring C++20, but the file is compiled in a target that does not directly link Extcovfie and therefore compiles at C++17. AlongStepCartMapFieldMscAction avoids this because it is placed in _celer_covfie_src which directly links Extcovfie and inherits cxx_std_20 from covfie's INTERFACE_COMPILE_FEATURES. Apply the same fix to AlongStepRZMapFieldMscAction." Assisted-by: Claude Sonnet 4.6
RZMapMagneticField.hh included RZMapField.hh, which when covfie is enabled pulls in covfie headers requiring C++20. This broke compilation of any downstream .cc file (e.g. celer-g4/DetectorConstruction.cc) that includes the header but is compiled at C++17. Apply the same adapter pattern used by CartMapMagneticField: introduce RZAdapterField, a plain struct whose operator() is defined in a new RZMapMagneticField.cc. The .cc file includes RZMapField.hh and is compiled as part of the accel library, which already links Extcovfie and therefore compiles at C++20. The .hh now only includes RZMapFieldParams.hh (which forward-declares RZMapFieldParamsData), exposing no covfie types to downstream users. Prompt: "DetectorConstruction.cc in celer-g4 includes RZMapMagneticField.hh which pulls in RZMapField.hh and covfie headers requiring C++20, but DetectorConstruction.cc is compiled at C++17. CartMapMagneticField.hh avoids this by using a CartAdapterField struct whose operator() is implemented in CartMapMagneticField.cc, keeping the covfie include out of the header. Apply the same pattern to RZMapMagneticField." Assisted-by: Claude Sonnet 4.6
Before this change, RZMapField had a separate Collection-based CPU implementation predating covfie, guarded by #if !CELERITAS_USE_COVFIE throughout. CartMapField already followed a cleaner pattern: a minimal stub in the data header, inline CELER_NOT_CONFIGURED stubs in the params header, and a one-line type alias in the field header. This commit brings RZMapField into alignment with that pattern, removing ~350 lines of duplicated guard logic. Changes: - Delete RZMapFieldParams.cc: its only content was non-covfie stubs, now replaced by inline stubs in RZMapFieldParams.hh inside a #if !(CELERITAS_USE_COVFIE || __DOXYGEN__) block, matching CartMapFieldParams.hh exactly (Impl struct, ImplDeleter, ctor, host_ref, device_ref all CELER_NOT_CONFIGURED or CELER_UNREACHABLE). - Add minimal RZMapFieldParamsData<W,M> stub in RZMapFieldData.hh's #else block (one field: FieldDriverOptions options) so the type is always a complete primary template, mirroring CartMapFieldData.hh. - Generalize detail::NotImplementedField from a Cart-specific class to a template<template<Ownership, MemSpace> class ParamsData> class, keeping using ParamsRef = NativeCRef<ParamsData> so FieldTrackPropagator<Field> can still access Field::ParamsRef as a concrete type. Remove the CartMapFieldData.hh include from NotImplementedField.hh (each field header includes its own data header instead). - Update CartMapField.hh to pass CartMapFieldParamsData explicitly: using CartMapField = detail::NotImplementedField<CartMapFieldParamsData> - Simplify RZMapField.hh's #else block to include RZMapFieldData.hh and alias: using RZMapField = detail::NotImplementedField<RZMapFieldParamsData> - Remove RZMapFieldParams.cc from src/celeritas/CMakeLists.txt SOURCES. Add AlongStepRZMapFieldMscAction to the covfie-OFF else() branch alongside AlongStepCartMapFieldMscAction so it links against the stubs. - Disable RZMapFieldTest via #define RZMapFieldTest DISABLED_RZMapFieldTest when !CELERITAS_USE_COVFIE (DISABLED_ GoogleTest pattern, matching how Cart tests are not gated but RZ tests would fail without a field map). - Disable SimpleCmsRZFieldAlongStepTest the same way in AlongStep.test.cc and gate its two CTest entries on CELERITAS_USE_covfie in test/celeritas/CMakeLists.txt to avoid the no-tests-written failure. - Remove non-bilinear Case 1 from Fields.test.cc interp_validation since the Collection-based interpolation path no longer exists; keep only the bilinear cross-term check (Case 2) and relax tolerance to 2e-7. Prompt: "RZMapField currently has a Collection-based CPU implementation predating covfie, with many #if CELERITAS_USE_COVFIE guards scattered across the field, data, params headers and test files. CartMapField follows a cleaner pattern with minimal stubs and no guards in most places. Remove the non-covfie RZMapField code path so that RZMapField matches the CartMapField pattern exactly: a minimal complete type stub in the data header's #else block, inline CELER_NOT_CONFIGURED stubs in the params header, and a one-line NotImplementedField type alias in the field header. Generalize detail::NotImplementedField to support both CartMapField and RZMapField without duplicating it. Use the DISABLED_ GoogleTest prefix trick (not full #if wrapping) to skip RZ tests when covfie is off, exactly as Cart tests are handled. Do not add guards in RZ test files where the analogous Cart code does not have them. Remove Case 1 from interp_validation in Fields.test.cc since the non-bilinear interpolation path is gone. Delete RZMapFieldParams.cc. Update CMakeLists to compile AlongStepRZMapFieldMscAction in the covfie-OFF branch and gate the SimpleCmsRZFieldAlongStep CTest entries on CELERITAS_USE_covfie." Assisted-by: Claude Sonnet 4.6
…Test The msc_rzfield test body contained a #if CELERITAS_USE_COVFIE guard with separate expected displacement and angle values for the non-covfie path. Since SimpleCmsRZFieldAlongStepTest is already disabled via the DISABLED_ prefix when covfie is OFF, the #else branch was dead code that could never execute. Remove the guard and retain only the covfie expected values. Prompt: "Remove remaining #if CELERITAS_USE_COVFIE guards from SimpleCmsRZFieldAlongStepTest test bodies. The whole fixture is disabled when covfie is OFF so any #else branches with non-covfie expected values are dead code and should be deleted." Assisted-by: Claude Sonnet 4.6
The displacement and angle expectations used EXPECT_SOFT_EQ (1e-12 relative tolerance) but CI hardware produces results with ~1e-8 relative deviation from the reference values, likely due to differing FMA instruction scheduling across GPU architectures. Switch to EXPECT_SOFT_NEAR with 1e-7 to accommodate this cross-platform variation. Prompt: "Relax the tolerance on the displacement and angle checks in SimpleCmsRZFieldAlongStepTest.msc_rzfield to accommodate floating-point variation across GPU hardware. The CI machine gives a result differing by ~1e-8 relative error from the reference value, which exceeds the default EXPECT_SOFT_EQ tolerance of 1e-12." Assisted-by: Claude Sonnet 4.6
esseivaju
reviewed
Apr 10, 2026
Member
|
The CI build error seems internal to covfie, we should probably submit a fix: |
sethrj
approved these changes
Apr 10, 2026
sethrj
left a comment
sethrj
left a comment
Member
There was a problem hiding this comment.
Looks great! Once we figure out that failing test, anyway…
Contributor
Author
|
@sethrj Just documenting here because it's relevant. The path we chose determines how we fail the SimpleCmsRZFieldAlongStepTest.msc_rzfield test here in single precision build whereas the previous non-covfie implementation succeeds by essentially hiding it. Should we adapt our implentation to follow the original rz behavior of returning zero field out of bounds? Covfie PathThe track is at The DormandPrince RK integrator evaluates the field at 7 substep positions: With Covfie clamps the out-of-bounds query to the nearest With and Non-Covfie PathIt does a initial check on bounds returning |
Contributor
Author
|
Now the second error in SimpleCmsRZFieldAlongStepTest.msc_rzfield_finegrid here, that actually happened in the original non-covfie implementation path as well but because the test passed the expected value test, it never got reported. In msc_rzfield_finegrid, the "failed to change position" error is a known float32 precision artifact — the step 6.1e-7 cm is below the ULP at r≈125 cm. This is logged silently in CI today because the test happens to pass with the non-covfie expected values. But it's a real error being swallowed. Unless we want to play around with the mfp, the right fix is probably to add a ScopedLogStorer (as done in LeadBoxAlongStepTest.position_change) so the test explicitly captures and expects those error messages in debug builds, rather than letting them pass through silently. This makes the known limitation visible and intentional instead of accidental. If the expected values are then updated to match the covfie interpolation output, the test would be both correct and self-documenting about the float32 precision edge case. |
Contributor
Author
@esseivaju acts-project/covfie#98 already did but nobody has reviewed it yet. |
Member
|
@rahmans1 Good debugging. I think a follow-on PR should impose a global limit on the maximum step size, and/or we should get an initial guess on the propagation step (e.g. using the safety distance or straight-line distance) before doing the field substep convergence. Regarding the failure (CC @esseivaju ): please From 85055ac41f9f850eb7926e5a2e6d586fff4fc48e Mon Sep 17 00:00:00 2001
From: Seth R Johnson <johnsonsr@ornl.gov>
Date: Sat, 11 Apr 2026 07:33:43 -0400
Subject: [PATCH] Work around covfie std usage
---
src/CMakeLists.txt | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 49b80b529..e19972275 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -166,6 +166,14 @@ if(CELERITAS_USE_covfie)
list(APPEND _covfie_libs $<COMPILE_ONLY:covfie::hip> Celeritas::ExtDeviceApi)
endif()
target_link_libraries(Extcovfie INTERFACE ${_covfie_libs})
+
+ if(CELERITAS_USE_CUDA AND covfie_VERSION VERSION_LESS_EQUAL 0.15.3)
+ # See https://github.com/acts-project/covfie/pull/98 :
+ # avoid warning/runtime failure due to use of std::clamp in device code
+ target_compile_options(Extcovfie
+ INTERFACE "$<$<COMPILE_LANGUAGE:CUDA>:--expt-relaxed-constexpr>"
+ )
+ endif()
endif()
#----------------------------------------------------------------------------#
--
2.53.0 |
In float32, the step length ~6e-7 cm is below machine epsilon at r~125 cm, causing all tracks to fail to change position in debug builds. Add a ScopedLogStorer to explicitly capture and expect these errors in float32+debug builds, and widen the angle tolerance for float32 builds. Prompt: "Add ScopedLogStorer to msc_rzfield_finegrid to explicitly handle float32 precision limit where step length is below machine epsilon, and widen angle tolerance for float32 builds" Assisted-by: Claude Sonnet 4.6
Store grid bounds from RZMapFieldInput in RZMapFieldParamsData and add a bounds check in RZMapField::operator(). Without this, covfie clamps out-of-bounds queries to boundary values, causing NaN in the RK error state when step length is infinite (vacuum track with no cross sections). Prompt: "Return zero outside field grid bounds in RZMapField: store min/max bounds from RZMapFieldInput in RZMapFieldParamsData and check them in RZMapField::operator()" Assisted-by: Claude Sonnet 4.6
sethrj
approved these changes
Apr 13, 2026
sethrj
left a comment
sethrj
left a comment
Member
There was a problem hiding this comment.
Awesome! Thanks for tracking down the bugs and getting the implementation switched over.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add covfie backend for RZMapField
Add a covfie-backed implementation of
RZMapFieldfor storage and bilinearinterpolation on the R-Z grid, with GPU acceleration via covfie's device
backends.
Core implementation
CovfieRZFieldTraitsdefining the 2D covfie pipeline for RZ maps:host path uses affine→clamp→linear→strided→array with
float2storage;device path uses the same pipeline with CUDA/HIP device array backends
(hardware texture interpolation was evaluated but switched to software
interpolation for consistency)
RZMapFieldData.covfie.hhwith four ownership/memspace specializationsand
RZMapField.covfie.hhthat evaluates the field at (r, z) and projectsBr onto Cartesian Bx/By
RZMapFieldParams.covfie.ccwith pimpl-based constructor that transposesfrom [Z][R] input stride to covfie's (R, Z) ordering
#if CELERITAS_USE_COVFIEdispatch intoRZMapFieldData.hh,RZMapField.hh, andRZMapFieldParams.hhRZAdapterFieldto hide covfie C++20 headers fromRZMapMagneticField.hhNon-covfie path removal
When covfie is enabled, the original
UniformGrid+Collectioncode path isremoved (matching the
CartMapFieldpattern):CELER_NOT_CONFIGUREDstubs inRZMapFieldParams.hhRZMapFieldParamsDatastub inRZMapFieldData.hh#elseblock
using RZMapField = detail::NotImplementedField<RZMapFieldParamsData>whencovfie is disabled;
DISABLED_SimpleCmsRZFieldAlongStepTestguards testsOut-of-bounds behavior
Return
{0, 0, 0}for positions outside the field grid, matching thenon-covfie convention. Without this, covfie's clamp behavior returns nonzero
boundary values at substep positions that can reach
r ~ 5e20 cmwhenstep_lengthis infinite (vacuum track with no cross sections), causing NaNin the RK error state.
Test updates
RZMapFieldTest.deviceverifying covfie field values on GPURZMapFieldTestSimpleCmsRZFieldAlongStepTestexpected values for covfie bilinearinterpolation
msc_rzfield_finegrid: step~6e-7 cmis below machine epsilon atr~125 cm; useScopedLogStorerto explicitly capture the resulting "failed to change position" errors in
float32+debug builds
Limitations and future work
CELERITAS_REAL_TYPE; full doublesupport requires templating
CovfieRZFieldTraitsonreal_typeand adouble-precision
convert_bfieldtool inacts-project/covfiebehavior; would require a dispatcher field class and mixed-field action
This PR is AI-assisted (Claude Code, claude-opus-4-6, claude-sonnet-4-6). Follows from discussion in #2298