Skip to content

Improve consistency of scintillation/Cherenkov import with Geant4#2303

Open
sethrj wants to merge 34 commits intoceleritas-project:developfrom
sethrj:scint-import
Open

Improve consistency of scintillation/Cherenkov import with Geant4#2303
sethrj wants to merge 34 commits intoceleritas-project:developfrom
sethrj:scint-import

Conversation

@sethrj
Copy link
Member

@sethrj sethrj commented Mar 9, 2026

CHAINED on #2306

This completely refactors the scintillation import data to use inp data structures and to load based on the availability of the geant4 processes.

Consequences:

  • Completely removed the rest of the particle scintillation data; we can restore later.
  • Scintillation process will only be added if any EM particle has a scintillation process.
  • No scintillation process will be added if no scintillating materials are present.
  • When setting up 'optical offload gen' problems, we will load EM particle types
  • Improved validation of scintillation material properties
  • Cherenkov and scintillation are set up based on optional OpticalPhysics::gen processes
  • Optical physics will be enabled if and only if any of gen/bulk/surface properties are present

Each scintillation component is now represented as a completely independent unnormalized time- and energy-dependent spectrum: it has a yield, energy distributions (independent distribution type [normal, grid] and quantity [wavelength, energy]), and time distribution.

sethrj added 15 commits March 7, 2026 07:29
Move all scintillation data loading from GeantImporter to GeantPhysicsLoader.
This removes ImportScintData and related structs from ImportOpticalMaterial,
loading directly into inp::ScintillationMaterial in the physics loader.

- Load scintillation properties per optical material in GeantPhysicsLoader
- Support up to 3 components with Gaussian or grid-based spectra
- Convert Gaussian approximations to inp::NormalDistribution
- Calculate per-component yield from yield_per_energy * yield_frac
- Remove all scintillation-related code from GeantImporter
- Remove ImportGaussianScintComponent, ImportScintComponent,
  ImportMaterialScintSpectrum, and ImportScintData structs

Assisted-by: GitHub Copilot (Claude Sonnet 4.5)
…intillationProcess

- Remove ScintillationParams::from_import static method
- Update constructor to take optical::MaterialParams and inp::ScintillationProcess
- Remove ImportScintData and related structs from ROOT dictionary
- Update Problem.cc and ImportedDataTestBase to use new constructor
- Rewrite Scintillation.test.cc with new inp:: types and manual optical material setup
- Update GeantImporter.test.cc to check inp::ScintillationProcess instead of ImportOpticalMaterial
- Fix ImportDataConverter to remove obsolete scintillation conversions
- Fix GeantPhysicsLoader yield calculation: component yields are relative weights, not absolute photons/MeV

The total yield is now correctly summed from individual component yields in ScintillationParams,
and component yield fractions are normalized during loading in GeantPhysicsLoader.

Assisted-by: GitHub Copilot (Claude Sonnet 4.5)
Extract helper functions to anonymous namespace:
- try_load_gaussian_spectrum: Load and validate mean/sigma pairs
- load_scintillation_component: Load single component spectrum
- normalize_component_yields: Normalize yields to material total

Validates that both lambda_mean and lambda_sigma are present together
for each prefix (CELER_ or deprecated unprefixed), catching
configuration errors where only one parameter is specified.

Assisted-by: GitHub Copilot (Claude Sonnet 4.5)
@sethrj sethrj added enhancement New feature or request physics Particles, processes, and stepping algorithms external Dependencies and framework-oriented features and removed external Dependencies and framework-oriented features labels Mar 9, 2026
@github-actions
Copy link

github-actions bot commented Mar 9, 2026

Test summary

 5 935 files   9 563 suites   8m 5s ⏱️
 1 757 tests  1 748 ✅   9 💤 0 ❌
31 070 runs  30 931 ✅ 139 💤 0 ❌

Results for commit 05a105f.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Mar 10, 2026

Codecov Report

❌ Patch coverage is 87.21805% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.69%. Comparing base (96e36e8) to head (05a105f).

Files with missing lines Patch % Lines
...c/celeritas/ext/detail/GeantScintillationLoader.cc 82.10% 11 Missing and 6 partials ⚠️
src/celeritas/ext/detail/GeantOpticalMatHelper.hh 69.23% 4 Missing ⚠️
src/celeritas/ext/GeantImporter.cc 80.00% 2 Missing and 1 partial ⚠️
src/celeritas/setup/Problem.cc 76.92% 1 Missing and 2 partials ⚠️
...eritas/optical/gen/detail/ScintSpectrumInserter.hh 96.55% 1 Missing and 1 partial ⚠️
src/celeritas/ext/detail/GeantPhysicsLoader.cc 93.75% 0 Missing and 1 partial ⚠️
src/celeritas/optical/gen/ScintillationData.hh 87.50% 0 Missing and 1 partial ⚠️
src/celeritas/optical/gen/ScintillationOffload.hh 75.00% 0 Missing and 1 partial ⚠️
src/celeritas/optical/gen/ScintillationParams.cc 95.00% 0 Missing and 1 partial ⚠️
src/celeritas/setup/StandaloneInput.cc 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2303      +/-   ##
===========================================
- Coverage    87.22%   85.69%   -1.53%     
===========================================
  Files         1357     1324      -33     
  Lines        43292    41705    -1587     
  Branches     13212    10818    -2394     
===========================================
- Hits         37761    35740    -2021     
- Misses        4317     4369      +52     
- Partials      1214     1596     +382     
Files with missing lines Coverage Δ
src/accel/SetupOptions.cc 73.86% <100.00%> (+0.30%) ⬆️
...eleritas/ext/detail/GeantMaterialPropertyGetter.hh 72.41% <ø> (+0.19%) ⬆️
src/celeritas/g4/SupportedOpticalPhysics.cc 93.75% <ø> (-0.28%) ⬇️
src/celeritas/inp/OpticalPhysics.hh 57.14% <100.00%> (+2.04%) ⬆️
src/celeritas/inp/Physics.hh 0.00% <ø> (-100.00%) ⬇️
src/celeritas/io/ImportOpticalMaterial.hh 80.00% <ø> (-8.10%) ⬇️
src/celeritas/io/ImportParameters.hh 100.00% <ø> (+3.22%) ⬆️
src/celeritas/io/detail/ImportDataConverter.cc 29.34% <ø> (+4.80%) ⬆️
src/celeritas/optical/Types.hh 100.00% <ø> (ø)
...rc/celeritas/optical/gen/ScintillationGenerator.hh 98.75% <100.00%> (-0.02%) ⬇️
... and 12 more

... and 715 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sethrj sethrj marked this pull request as draft March 10, 2026 12:33
sethrj added 7 commits March 10, 2026 08:40
…PhysicsLoader

Introduce GeantOpticalMatHelper, analogous to GeantSurfacePhysicsHelper,
that bundles an OptMatId, G4Material const*, and a
GeantMaterialPropertyGetter for convenient per-material property access.

Extract the scintillation loading logic from GeantPhysicsLoader::scintillation
into a new functor class GeantScintillationLoader, constructed with a
reference to inp::ScintillationProcess.  The two anonymous-namespace helpers
load_scintillation_gaussian and load_scintillation_spectrum become private
static methods load_gaussian and load_spectrum on the new class.

GeantPhysicsLoader::scintillation is now a thin loop that builds
GeantOpticalMatHelper objects and delegates to GeantScintillationLoader.

Assisted-by: GitHub Copilot (Claude Sonnet 4.6)
…Helper

Define inline operator<< for both helper classes so they can be streamed
directly into log messages.  GeantOpticalMatHelper prints the G4Material name
and optical id; GeantSurfacePhysicsHelper prints the surface name and surface
id.

Update the error-context log messages in GeantScintillationLoader and
GeantSurfacePhysicsLoader to use the new operators, and reorganise the
surface-loader message to include finish/type alongside the model name.

Assisted-by: GitHub Copilot (Claude Sonnet 4.6)
…l mat/surface helpers

Add operator bool (checks non-null construction), a public
property_getter() accessor returning GeantMaterialPropertyGetter const&,
and CELER_ASSERT(*this) guards on member accessors, to both
GeantOpticalMatHelper and GeantSurfacePhysicsHelper.

Rename the get_property() accessor on GeantOpticalMatHelper to
property_getter() for consistency with the surface helper.  Update
GeantScintillationLoader and GeantPhysicsLoader to use the renamed
accessor.

Update operator<< on both helpers to stream via the property getter
(which already knows the material/surface name) instead of calling
GetName() directly.

Assisted-by: GitHub Copilot (Claude Sonnet 4.6)
@sethrj sethrj marked this pull request as ready for review March 10, 2026 14:56
@sethrj sethrj marked this pull request as draft March 10, 2026 14:56
@sethrj sethrj marked this pull request as ready for review March 11, 2026 08:54
@sethrj sethrj marked this pull request as draft March 11, 2026 08:57
@sethrj sethrj marked this pull request as ready for review March 12, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request physics Particles, processes, and stepping algorithms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant