Skip to content

feat(output): publish EventHeader from every generator source - #94

Merged
matclim merged 1 commit into
mainfrom
pr/event-header
Aug 13, 2026
Merged

feat(output): publish EventHeader from every generator source#94
matclim merged 1 commit into
mainfrom
pr/event-header

Conversation

@matclim

@matclim matclim commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Checklist


Stack created with GitHub Stacks CLIGive Feedback 💬

Summary by CodeRabbit

  • New Features

    • Added event metadata and provenance information to simulation output files.
    • Particle sources can now provide event headers; empty headers are supplied when unavailable.
    • Both Monte Carlo and full simulation outputs now include event header data.
  • Bug Fixes

    • Preserved complete-event filtering for full simulation outputs when no detector hits are present.
  • Documentation

    • Documented event metadata headers and provenance fields.

@coderabbitai

coderabbitai Bot commented Aug 13, 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: edb156db-230c-4f76-86fc-ca63c19bd169

📥 Commits

Reviewing files that changed from the base of the PR and between d6e7893 and 87ba937.

⛔ Files ignored due to path filters (1)
  • pixi.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • pixi.toml
  • src/mc_particle_source.hpp
  • src/sim_output_module.cpp

📝 Walkthrough

Walkthrough

The change raises the shipdatamodel requirement and adds SHiP::EventHeader production, propagation, and persistence for MC-only and full-simulation outputs.

Changes

Event header support

Layer / File(s) Summary
Provider event-header contract
pixi.toml, src/mc_particle_source.hpp
The dependency requirement is raised to >=0.4. The particle source helper adds optional event-header generation and publishes a default header when no generator is supplied.
MC output header writing
src/sim_output_module.cpp
The MC-only RNTuple stores and writes SHiP::EventHeader. Its input family now requires the event-header product.
Full-simulation header writing
src/sim_output_module.cpp
The full-simulation RNTuple stores and writes SHiP::EventHeader. Empty-hit filtering remains before pool acquisition.

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

Mergeability Score: ⚪ Minimal · up to 87ba9

This localized change has no actionable merge-blocking risk in the supplied evidence and is merge-ready after normal checks and review.

Suggested reviewers: olantwin

Sequence Diagram(s)

sequenceDiagram
  participant ParticleSource
  participant EventHeaderProduct
  participant MCOutputWriter
  participant SimOutputWriter

  ParticleSource->>EventHeaderProduct: publish SHiP::EventHeader
  EventHeaderProduct->>MCOutputWriter: provide event_header with mc_particles
  MCOutputWriter->>MCOutputWriter: bind header and write RNTuple entry
  EventHeaderProduct->>SimOutputWriter: provide event_header with mc_particles and sim_results
  SimOutputWriter->>SimOutputWriter: filter empty hits, bind header, and write entry
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title clearly describes the primary change: publishing EventHeader from every generator source.
Description check ✅ Passed The description includes the required Checklist section and confirms all listed checks are complete.
✨ 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 pr/event-header

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.

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown

✅ prek hooks passed

@github-actions

Copy link
Copy Markdown

Physics Metrics Comparison

All configurations match reference (no significant differences)

  • Configurations compared: 1
  • Matching reference: 1

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 olantwin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make prek happy. Apart from that, this looks good for merging.

@olantwin

Copy link
Copy Markdown
Contributor

Some other generators will probably need updating, e.g. genie and the file sources. I'll do those in a follow-up PR.

@matclim
matclim merged commit 00a30db into main Aug 13, 2026
7 checks passed
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.

2 participants