71 binary output names - #72
Merged
Merged
Conversation
…e structure and summary details
…ptions and filename conventions
…file in JSON output
…n parameters, and adjust output file naming conventions
…id, run, and detailed photon timing information
…reconstruction results
…rement ID, and run parameters
…ttings and photon timing details
…ton timing, clustering settings, and updated variable names
…improved output path handling
…n output structure
Closed
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes unpacker + photon reconstruction outputs by introducing explicit measurement_id/run provenance, updating summary JSON schemas, and aligning file/directory naming conventions across Python runner, C++ binaries, tests, eval configs, and architecture docs.
Changes:
- Add
--measurement-idand--runCLI requirements for the unpacker and photon clusterer when writing outputs; propagate into summary JSON via a newmeasurement_infoblock. - Rename/reshape summary JSON fields (e.g.,
parquet→output_parquet, sorting diagnostics, photon reconstruction counters, and new parquet/timing/settings echo sections). - Standardize output locations and filenames (logs subdirs, parquet naming patterns) and update tests/evals/docs accordingly.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/hermes/state/models/test_hermes_tpx3_spidr.py | Updates unit fixtures/assertions for new summary schemas (measurement_info, output_parquet, photon summary structure). |
| tests/unit/hermes/runner/analysis/hermes/test_unpacker.py | Adapts unpacker runner tests to new CLI args and summary format; removes several older execution-path tests. |
| tests/unit/hermes/runner/analysis/hermes/test_unpacker_integration.py | Updates integration expectations for new summary locations and parquet path semantics (now full paths). |
| tests/unit/hermes/runner/analysis/hermes/test_photon_reconstruction.py | Updates reconstruction command/skip-marker tests for new naming + CLI args; execution tests removed. |
| tests/unit/hermes/runner/analysis/hermes/test_event_reconstruction.py | Removes execution/status-flow tests (file now focuses on resolve/command helpers). |
| tests/unit/examples/analysis/test_two_stage.py | Updates example state construction to match renamed photon count fields. |
| src/hermes/state/models/analysis/hermes_tpx3_spidr.py | Extends/renames Pydantic models for unpacker + photon reconstruction summaries; adds stricter cross-field validation. |
| src/hermes/runner/analysis/hermes/unpacker.py | Adds measurement/run to CLI, updates summary path, validates new parquet naming/path rules, and updates output validation logic. |
| src/hermes/runner/analysis/hermes/run.py | Threads measurement_info through unpacking and photon reconstruction execution; adds best-effort output path on failures. |
| src/hermes/runner/analysis/hermes/photon_reconstruction.py | Changes summary/output path derivation and CLI construction for new reconstruction binary interface. |
| src/backends/unpackers/tpx3-spidr/cpp/tests/workflow_tests.cpp | Updates C++ workflow tests for new logs dir, schema keys, and parquet filename patterns/paths. |
| src/backends/unpackers/tpx3-spidr/cpp/src/unpacker.cpp | Updates log directory and overwrite-detection to match new underscore-based file naming. |
| src/backends/unpackers/tpx3-spidr/cpp/src/tpx3SpidrUnpacker.cpp | Adds --measurement-id/--run parsing/requirements and updates help text + workflow invocation. |
| src/backends/unpackers/tpx3-spidr/cpp/src/summary_json.cpp | Prepends measurement_info + inputfile, renames parquet block, and updates sorting timing field. |
| src/backends/unpackers/tpx3-spidr/cpp/src/parquet_writer.cpp | Implements new parquet filename scheme and records full output paths in diagnostics. |
| src/backends/unpackers/tpx3-spidr/cpp/inc/unpacker.h | Extends runTwoPassWorkflow signature with measurement/run identifiers. |
| src/backends/unpackers/tpx3-spidr/cpp/inc/summary_json.h | Extends summary-json content struct with measurement/run/inputfile fields. |
| src/backends/reconstruction/photons/cpp/tests/summary_writer_tests.cpp | Updates photon reconstruction summary tests for new schema blocks and renamed counters/paths. |
| src/backends/reconstruction/photons/cpp/src/summary_writer.cpp | Implements the expanded photon reconstruction summary JSON structure (identity, settings echo, timing, parquet paths). |
| src/backends/reconstruction/photons/cpp/src/photonClusterer.cpp | Updates CLI (--output as analysis dir + required measurement/run) and rewrites output/summary path derivation. |
| src/backends/reconstruction/photons/cpp/inc/summary_writer.h | Expands summary content structure to carry identity, settings/timing, and parquet path provenance. |
| examples/analysis/two_stage/run_two_stage.py | Updates example aggregation to use renamed photon count fields. |
| evals/cases/01-unpacking/input/config.yaml | Updates eval config paths to new relative directory conventions. |
| evals/cases/02-two-stage/input/config.yaml | Updates eval config paths to new relative directory conventions. |
| docs/architecture/unpacker.md | Documents new CLI requirements, directory layout, parquet naming, and new summary schema. |
| docs/architecture/photon_reconstruction.md | Documents new output directories/filenames and expanded reconstruction summary structure. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+357
to
361
| if parquet_path in listed_files: | ||
| raise HermesTpx3OutputError( | ||
| f"summary lists the same Parquet file more than once: " | ||
| f"{relative_path}" | ||
| f"{parquet_path}" | ||
| ) |
Comment on lines
+300
to
304
| photon_output_file = | ||
| (photons_dir / (pixel_name.raw_file_stem + "_photon_" + | ||
| pixel_name.part_index + ".parquet")) | ||
| .string(); | ||
| pixels_output_file = |
Comment on lines
308
to
+311
| summary_path = | ||
| (logs_dir / (stem + "-reconstruction-summary.json")).string(); | ||
| (logs_dir / (pixel_name.raw_file_stem + | ||
| "_photon_reconstruction_summary.json")) | ||
| .string(); |
Comment on lines
179
to
+184
| def test_command_passes_named_flags_and_settings(tmp_path: Path) -> None: | ||
| analysis = _analysis(tmp_path, "run_000000-chip-0-part-00000.parquet") | ||
| analysis = _analysis(tmp_path, "run_000000_chip_0_pixels_00000.parquet") | ||
| reconstruction = analysis.photon_reconstruction | ||
| analysis_root = tmp_path / "analysis" | ||
| input_file = FileReference( | ||
| path=tmp_path / "analysis" | ||
| path=analysis_root |
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
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.
This pull request updates the documentation, configuration, and C++ implementation for the unpacker and photon reconstruction pipeline to clarify and standardize file naming, directory structure, and summary output formats. It introduces explicit measurement and run identifiers throughout the workflow, improves the clarity of output locations, and aligns the summary JSON structure and command-line interfaces with these conventions.
Key changes include:
Documentation and Output Structure Standardization
Updated documentation in
photon_reconstruction.mdandunpacker.mdto clarify directory structures, file naming conventions, and the inclusion of measurement/run identifiers in both unpacker and reconstruction outputs. This includes new directory paths (e.g.,analysis/logs/photon_reconstruction/,analysis/logs/unpacking/) and standardized filename patterns for Parquet and summary files. [1] [2] [3]The summary JSON files for both unpacker and reconstruction now open with a
measurement_infosection, includingmeasurement_idandrun, and all file paths are specified exactly as written, so downstream tools can locate outputs directly. [1] [2]Command-Line Interface and Configuration
Both the unpacker and photon reconstruction binaries now require
--measurement-idand--runarguments when writing outputs, and these are copied into the summary JSON for provenance. The help message and documentation have been updated accordingly. [1] [2]Updated example and test configuration files (
config.yaml) to use new relative directory conventions and reflect the simplified structure. [1] [2]Photon Reconstruction Summary and Output
The photon reconstruction summary structure has been expanded to include measurement/run identity, detailed clustering settings, explicit input/output file paths, and updated count field names for clarity (e.g.,
pixels_read,clusters_formed,total_photons,rejected_clusters). [1] [2] [3]Output Parquet files for photons and pixel clusters are now written to
photons/andpixel_clusters/directories, with filenames following the new conventions. [1] [2]Minor Consistency and Clarity Improvements
Updated field names and descriptions throughout the documentation and code to match the new conventions and improve clarity (e.g.,
photon_pixels→pixel_clusters,components_formed→clusters_formed). [1] [2]Help messages and documentation have been revised to clearly describe all required and optional command-line arguments, their effects, and the new output locations.
These changes ensure that all outputs are traceable to their measurement/run context, that file locations are predictable and machine-readable, and that the workflow is easier to use and maintain.