Fix silent data loss in the Prometheus pipeline: raise on missing tables, sync TRUTH.PROMETHEUS#911
Open
sevmag wants to merge 4 commits into
Open
Conversation
An extractor whose table is absent from the input file (e.g. a non-default Prometheus photon_field_name, or a typo'd table_name) was silently skipped, producing truth-only databases with no diagnostic. Validate all configured tables once per file and raise a ValueError naming the missing and available tables. Part of graphnet-team#909. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TRUTH.PROMETHEUS still described a truth schema (injection_*, primary_lepton_1_*, ...) that Prometheus stopped writing long ago; using it against a freshly converted file silently strips every truth column. Rename that list to TRUTH.PROMETHEUS_LEGACY (it matches older files such as the bundled example database, which the 04_training examples run on) and set TRUTH.PROMETHEUS to the schema Prometheus writes today. PrometheusTruthExtractor now takes its default columns from TRUTH.PROMETHEUS so the constant and the extractor cannot drift apart again. Part of graphnet-team#909. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The default (TRUTH.PROMETHEUS) covers the event-level scalars every Prometheus injection type writes; a columns override lets users add injection-specific fields such as LeptonInjector's bjorken_x/bjorken_y/ column_depth without subclassing. Part of graphnet-team#909. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The datamodule tests run on the bundled legacy example database, so they need TRUTH.PROMETHEUS_LEGACY; PublicPrometheusDataset duplicated the current schema as a literal list and now derives it from TRUTH.PROMETHEUS instead. Part of graphnet-team#909. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Closes #909.
Both failure modes in the Prometheus conversion/training path were silent — truth-less or photon-less datasets with no error. This PR makes the first one loud and fixes the second at its root.
Changes
PrometheusReader: raise on missing tables. The reader dispatched extractors by exact table-name match and silently skipped any extractor whose table wasn't in the file (e.g. a non-defaultphoton_field_namein the Prometheus config, or a typo'dtable_name). It now validates all configured tables once per file and raises aValueErrorlisting the missing and the available tables.TRUTH.PROMETHEUSsynced with current Prometheus output. The constant still described a truth schema (injection_*,primary_lepton_1_*, ...) that Prometheus stopped writing long ago; against a freshly converted file,Dataset._remove_missing_columnssilently stripped every truth column. Now:TRUTH.PROMETHEUS= the event-level truth written by every current Prometheus injection type (interaction+initial_state_*), matchingInjection.to_dict().TRUTH.PROMETHEUS_LEGACYfor older files such as the bundled example database (prometheus-events.db).PrometheusTruthExtractorderives its default columns fromTRUTH.PROMETHEUS, so the constant and the extractor can no longer drift apart.PrometheusTruthExtractorgained an optionalcolumnsoverride for truth fields that only some injection types write (e.g. LeptonInjector'sbjorken_x,bjorken_y,column_depth— deliberately not in the default, since GENIE-based files don't have them).examples/04_trainingscripts and the datamodule tests (which run on the bundled legacy database) usePROMETHEUS_LEGACY;PublicPrometheusDataset._event_truthnow derives fromTRUTH.PROMETHEUSinstead of duplicating it.Breaking changes
TRUTH.PROMETHEUSchanges meaning: code using it against old databases must switch toTRUTH.PROMETHEUS_LEGACY. This is intentional — the default-looking name should describe what current Prometheus writes.PrometheusReaderinstead of quietly converting to truth-only databases. Note this includes files from runs where zero events produced hits (Prometheus writesmc_truth-only files in that case) — previously these converted silently; failing loudly surfaces e.g. vertices injected outside the detector volume.Testing
tests/data/test_prometheus_reader.py: extraction of both tables from the shipped Prometheus test parquet, verification that everyTRUTH.PROMETHEUSfield exists in that (current-schema) file, thecolumnsoverride, and the missing-table raise.tests/data/test_datamodule.py+ the new tests: 14 passed locally; pre-commit (black, flake8, docformatter, pydocstyle, mypy) green on all touched files.🤖 Generated with Claude Code