Skip to content

feat: type the GENIE and particle-gun boundaries#73

Merged
olantwin merged 2 commits into
mainfrom
feat/units-readers
Jul 24, 2026
Merged

feat: type the GENIE and particle-gun boundaries#73
olantwin merged 2 commits into
mainfrom
feat/units-readers

Conversation

@olantwin

@olantwin olantwin commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Stacked on #72. GENIE SI→canonical conversions become derived quantities (exact powers of ten); gun config is typed via get_quantity with the Philox stream untouched. 100-event gun_only output bitwise identical to origin/main.

Summary by CodeRabbit

  • Improvements
    • Added unit-aware handling for beam energies, distances, angles, momenta, lifetimes, and event coordinates.
    • Configuration values now accept explicit physical units, reducing ambiguity and preventing unit-conversion errors.
    • Updated particle generation, fixed-target interactions, Pythia8 simulations, and GENIE event conversion to use consistent units.
    • Improved benchmark configuration with correctly scaled energy and lifetime values.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 97a9f5c6-f6f5-4dd4-8985-dcf7d7960876

📥 Commits

Reviewing files that changed from the base of the PR and between c5d0220 and 9218e28.

📒 Files selected for processing (7)
  • CMakeLists.txt
  • benchmarks/pythia8_benchmark.cpp
  • src/fixed_target_source.cpp
  • src/genie_reader_source.cpp
  • src/particle_gun_source.cpp
  • src/pythia8_source.cpp
  • src/pythia_common.hpp

📝 Walkthrough

Walkthrough

The PR replaces raw generator configuration scalars with SHiP unit quantities across particle gun, fixed-target, Pythia8, GENIE, and benchmark paths. Shared Pythia helpers perform explicit unit conversions, and required SHiP unit libraries are added to plugin and benchmark linkage.

Changes

Unit-aware event generation

Layer / File(s) Summary
Pythia unit contracts
src/pythia_common.hpp
Beam energy, lifetime thresholds, extraction offsets, and time conversion use typed SHiP quantities.
Source quantity integration
src/particle_gun_source.cpp, src/fixed_target_source.cpp, src/pythia8_source.cpp, src/genie_reader_source.cpp, CMakeLists.txt
Source configuration and generation paths use explicit momentum, angle, energy, length, and time units, with required unit-library linkage.
Benchmark unit wiring
benchmarks/pythia8_benchmark.cpp, CMakeLists.txt
Benchmark beam and lifetime arguments use typed units, and the executable links against SHiP units.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • ShipSoft/aegir#34: Earlier source and wiring changes overlap the updated event generator implementations.
  • ShipSoft/aegir#38: Shared Pythia helpers and benchmark paths are modified in both PRs.
  • ShipSoft/aegir#72: Directly overlaps the unit typing and Pythia/CMake linkage changes.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description omits the required checklist sections and doesn't cover commit message, lint, or CI status. Rewrite it using the repo template and add the checklist items, including commit message, pixi run lint, and CI status.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and matches the PR's main focus on typed GENIE and particle-gun boundaries.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/units-readers

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@olantwin
olantwin force-pushed the feat/units-pythia8 branch from 2952d72 to e9ec269 Compare July 10, 2026 09:28
@olantwin
olantwin force-pushed the feat/units-readers branch from 482d5c8 to 319561a Compare July 10, 2026 09:28
@olantwin
olantwin marked this pull request as ready for review July 14, 2026 14:51
@olantwin
olantwin force-pushed the feat/units-readers branch from 319561a to f0ec9d8 Compare July 14, 2026 16:48
@olantwin
olantwin force-pushed the feat/units-pythia8 branch from e9ec269 to 52e61e0 Compare July 14, 2026 16:48
olantwin added 2 commits July 22, 2026 16:12
Pythia8's native units (GeV, mm, mm/c production time) become explicit:
beam energies are ship::Energy, lifetime thresholds and tProd are mm/c
quantities, and the mm/c -> ns conversion derives from the exact SI
definition of c instead of a hand-typed 299.792458. Fixed-target and
Pythia8 source configuration is read via get_quantity, so each
parameter's unit is defined in exactly one place.

The only numeric change is the tProd conversion (division by 299.792458
becomes multiplication by the c-derived factor): at most 1 ulp on ~20%
of values. Validation histograms of a 200-event pythia8_only run are
bitwise identical to an origin/main build.
GENIE's SI vertex (m, s) converts to the canonical mm/ns through
quantities instead of hand-typed 1e3/1e9 factors (exact powers of ten,
value-identical). Particle-gun configuration is read via get_quantity
(momenta in GeV/c, angle in rad, vertex in mm); sampling stays on raw
numbers at the RNG call so the Philox stream is bit-identical.

Validation: determinism check passes; a 100-event gun_only run is
bitwise identical to an origin/main build (order-independent event
comparison; the parallel writer emits events in arbitrary order).
@olantwin
olantwin force-pushed the feat/units-pythia8 branch from 52e61e0 to cba7262 Compare July 22, 2026 14:14
@olantwin
olantwin force-pushed the feat/units-readers branch from f0ec9d8 to 9218e28 Compare July 22, 2026 14:14
Base automatically changed from feat/units-pythia8 to main July 23, 2026 07:43
@olantwin olantwin closed this Jul 24, 2026
@olantwin olantwin reopened this Jul 24, 2026
@github-actions

Copy link
Copy Markdown

Physics Metrics Comparison

1 configuration(s) have differences

  • Configurations compared: 1
  • With differences: 1

Configurations with Differences

comparison_physics-smoke

  • Files compared: 1
  • Histograms compared: 8
  • Fits compared: 0
  • Total differences: 4
Show differences
gun_st_nofield_smoke_validation.root:
  h_hit_edep.integral: 919 → 969 (Δ=5.441%)
  h_hit_z.integral: 919 → 969 (Δ=5.441%)
  h_hit_detector.integral: 919 → 969 (Δ=5.441%)
  h_hit_momentum.integral: 919 → 969 (Δ=5.441%)


About this comparison

Physics metrics are extracted from ROOT files and compared.
Default comparison uses 5% tolerance for histogram statistics
and 3σ for fit parameters.

Metrics stored in git notes: refs/notes/ci/physics-metrics/<config>

@olantwin

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@olantwin
olantwin merged commit 9db189c into main Jul 24, 2026
7 checks passed
@olantwin
olantwin deleted the feat/units-readers branch July 24, 2026 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant