Add helpers for array type, precision, and quantity conversion#2223
Open
sethrj wants to merge 22 commits intoceleritas-project:developfrom
Open
Add helpers for array type, precision, and quantity conversion#2223sethrj wants to merge 22 commits intoceleritas-project:developfrom
sethrj wants to merge 22 commits intoceleritas-project:developfrom
Conversation
This reverts commit 878e4c8.
sethrj
requested review from
drbenmorgan,
elliottbiondo,
mrguilima and
stognini
as code owners
sethrj
added
enhancement
sethrj
requested review from
amandalund
and removed request for
drbenmorgan,
elliottbiondo,
mrguilima and
stognini
There was a problem hiding this comment.
Pull request overview
This PR refactors Geant4–Celeritas interop to separate concerns of type conversion, precision casting, and unit/quantity handling, and to provide clearer, safer helpers for arrays and quantities. It also migrates several geometry, field, and primary-generation components to use these new helpers and quantity-based CLHEP conversions, with accompanying unit tests.
Changes:
- Introduces
to_array,to_g4vector,static_array_cast, andArrayQuantityutilities to make array-type and quantity conversions explicit and reusable across the codebase. - Refactors Geant4 geometry and transport adapters (e.g.,
GeantGeoTrackView,LocalTransporter, navigation updaters, and ORANGE transformers) to use the new helpers and quantity-based CLHEP conversion types instead of overloadedconvert_{to,from}_geant. - Updates VecGeom interop, field map builders, primary generators, and tests to rely on the new conversion APIs, and removes the old VecGeom compatibility shim.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/orange/g4org/Transformer.test.cc | Adjusts tests to use to_array, to_g4vector, and explicit rotation-matrix element checks matching the new Transformer implementation. |
| test/corecel/math/Quantity.test.cc | Adds tests for ArrayQuantity helpers (array construction, native_value_from, value_as) to validate array-of-quantity behavior. |
| test/corecel/cont/Array.test.cc | Adds tests for to_array (C-array to Array) and static_array_cast (array element type casting). |
| test/celeritas/Units.test.cc | Extends CLHEP unit tests to verify new quantity-based convert_{to,from}_geant overloads for fields. |
| test/accel/UserActionIntegration.test.cc | Switches point-distribution setup to use static_array_cast<double> for position arrays. |
| test/accel/TrackingManagerIntegration.test.cc | Same static_array_cast<double> update for several primary shape definitions in integration tests. |
| test/accel/IntegrationTestBase.cc | Updates primary-input helper shapes to use static_array_cast<double> for consistency with new array utilities. |
| test/accel/HepMC3PrimaryGenerator.test.cc | Simplifies momentum direction extraction using to_array on G4ThreeVector. |
| src/orange/g4org/Transformer.hh | Refactors Transformer to build rotation matrices directly from G4RotationMatrix/G4Transform3D components and drops the old convert_from_geant free functions. |
| src/orange/g4org/SolidConverter.cc | Uses to_array and static_array_cast to get Real3 normals from Geant4 vectors in cut-tube conversion. |
| src/geocel/vg/detail/VecgeomCompatibility.hh | Removes the old VecGeom compatibility helper in favor of centralized conversions in VecgeomTypes.hh. |
| src/geocel/vg/VecgeomTypes.hh | Adds to_vgvector and to_array VecGeom conversion helpers for Array↔Vector3D interop. |
| src/geocel/vg/VecgeomTrackView.hh | Replaces use of detail::to_vector with to_vgvector and simplifies VecGeom navigation calls accordingly. |
| src/geocel/vg/VecgeomParams.cc | Uses new to_array helper to convert VecGeom bounding boxes into Celeritas BBox. |
| src/geocel/g4/GeantTypes.hh | Introduces to_array, to_g4vector, and an axpy helper for G4ThreeVector to centralize Geant4 vector interop. |
| src/geocel/g4/GeantGeoTrackView.hh | Refactors Geant4 geometry track view to use quantity-based CLHEP-length conversions and to_array/to_g4vector for positions, directions, and normals. |
| src/geocel/g4/Convert.hh | Replaces generic unit-scaled convert_{to,from}_geant overloads with quantity-based templates and modernizes the remaining CLHEP-unit helpers. |
| src/geocel/detail/LengthUnits.hh | Cleans up includes; foundational length-unit constants remain unchanged. |
| src/geocel/detail/LengthQuantities.hh | Adds length quantity types (Centimeter, Millimeter) and aliases (CmLength, ClhepLength) plus array-quantity plumbing for geometry conversions. |
| src/geocel/GeantGeoParams.hh | Clarifies documentation for get_clhep_bbox to state units and precision explicitly. |
| src/geocel/GeantGeoParams.cc | Reworks world bounding-box conversion to use ClhepLength and make_quantity_array instead of raw convert_from_geant scaling. |
| src/corecel/random/distribution/DistributionInserter.cc | Switches array_cast usages to static_array_cast when storing position distributions. |
| src/corecel/math/ArrayUtils.hh | Removes the now-redundant array_cast in favor of the new static_array_cast in Array.hh. |
| src/corecel/math/ArrayQuantity.hh | New helper header implementing make_quantity_array, array native_value_from/native_value_to, and value_as for arrays of quantities. |
| src/corecel/math/ArrayOperators.hh | Makes array arithmetic operators constexpr and leaves semantics unchanged. |
| src/corecel/cont/Array.hh | Adds to_array for C-array conversion and static_array_cast for array element casting; retains core Array implementation. |
| src/celeritas/phys/PrimaryGenerator.cc | Updates primary generator to use static_array_cast for position and direction distributions; adjusts error messages slightly. |
| src/celeritas/field/UniformFieldParams.cc | Converts uniform field strength handling to use make_quantity_array<FieldTesla> and array native_value_from. |
| src/celeritas/ext/detail/NaviTouchableUpdater.cc | Uses quantity-based length conversion and to_g4vector for Geant4 navigation state setup while leaving existing CLHEP-scaled step conversion in place. |
| src/celeritas/ext/detail/HitProcessor.cc | Continues to use CLHEP-based conversions for setting Geant4 step data; adds a more descriptive log message wrapper for logical volumes (but currently contains a syntax issue in the validation macro). |
| src/celeritas/ext/GeantUnits.hh | Marks CLHEP field/time scaling constants as deprecated in favor of new quantity aliases and retains MeV conversion helper. |
| src/celeritas/Quantities.hh | Adds quantity aliases in the units namespace for CLHEP-based length, field, and time to support the new conversion templates. |
| src/accel/PGPrimaryGeneratorAction.cc | Updates primary generation to use convert_to_geant<ClhepLength> for positions and to_g4vector(static_array_cast<double>) for directions. |
| src/accel/LocalTransporter.cc | Uses static_array_cast and to_array for momentum directions and quantity-based CLHEP length/time conversions for positions and times. |
| src/accel/HepMC3PrimaryGenerator.cc | Simplifies direction setup using Array deduction and to_g4vector. |
| src/accel/CylMapMagneticField.cc | Switches cylindrical field map construction to quantity-based length and field conversions instead of raw CLHEP scaling. |
| src/accel/CartMapMagneticField.cc | Similar quantity-based refactor for Cartesian field map input, both in grid extents and field values. |
| app/celer-g4/SensitiveDetector.cc | Adjusts includes toward using GeantUnits but still relies on CLHEP energy/time conversions when populating hits. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Test summary 5 867 files 9 390 suites 17m 44s ⏱️ Results for commit 45a4fec. ♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #2223 +/- ##
=========================================
Coverage 86.83% 86.83%
=========================================
Files 1325 1328 +3
Lines 41904 41950 +46
Branches 12930 13385 +455
=========================================
+ Hits 36387 36429 +42
- Misses 4312 4498 +186
+ Partials 1205 1023 -182
🚀 New features to boost your workflow:
|
amandalund
approved these changes
Feb 5, 2026
Comment on lines
+92
to
+93
| hit.energy_dep = units::ClhepEnergy{edep}.value(); | ||
| hit.time = native_value_from(units::ClhepTime{pre_step->GetGlobalTime()}); |
Contributor
There was a problem hiding this comment.
Maybe the former should also be native_value_from (just to make it clearer to developers that it's converting units) even though we know the energy units are both MeV.
| Array<G4double, 4> const&, | ||
| real_type cur_bfield[3]) { | ||
| auto bfield_native = convert_from_geant(bfield.data(), clhep_field); | ||
| using ClhepField = Quantity<units::ClhepUnitBField, double>; |
Contributor
There was a problem hiding this comment.
Use ClhepField from Quantities.hh?
Member
Author
There was a problem hiding this comment.
Not the same: real_type vs Geant4 value (double ).
Contributor
There was a problem hiding this comment.
The ones in Quantities are double too, were they meant to be real_type?
Comment on lines
+97
to
+99
| * Convert an array of quantities to native values. | ||
| * | ||
| * This applies native_value_from element-wise to each component. |
Contributor
There was a problem hiding this comment.
Suggested change
| * Convert an array of quantities to native values. | |
| * | |
| * This applies native_value_from element-wise to each component. | |
| * Get the values from an array of quantities. | |
| * | |
| * This applies value_as element-wise to each component. |
| //! \name Length quantity aliases | ||
|
|
||
| using CmLength = RealQuantity<Centimeter>; | ||
| using ClhepLength = Quantity<Millimeter, double>; |
Contributor
There was a problem hiding this comment.
We also have a ClhepLength alias in celeritas::units
Member
Author
There was a problem hiding this comment.
Yea, the separate namespace here (lengthunits) is to allow inclusion in geocel: that was introduced a while back. I thought there was a good reason to not make the celeritas/Units directly alias lengthunits but I can check.
|
|
||
| //---------------------------------------------------------------------------// | ||
| // FREE FUNCTIONS | ||
| //! Convert via a quantity to native Geant4 types/units |
Contributor
There was a problem hiding this comment.
Update description here and convert_from_geant below
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
2 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.
The existing
convert_{to/from}_geantfunctions are rather confusing because they do three things:To avoid confusion and errors i.e. the ones I originally thought were present in #2218, this defines new helper functions:
to_arrayandto_g4vectorfor type conversionarray_casttostatic_array_castto clarify what exactly is being cast (similar to the standard library)make_array_quantityand related function for creatingArray<Quantity<...>>that allows more expressive and type-safe conversion.