feat(pmp): key reference identity on reference_source_id and emit kind#775
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (60)
✅ Files skipped from review due to trivial changes (17)
🚧 Files skipped from review as they are similar to previous changes (41)
📝 WalkthroughWalkthroughThis PR adds first-class ChangesPMP kind/reference_source_id migration
Sequence Diagram(s)sequenceDiagram
participant Bundle as CMECMetric bundle
participant Walker as _walk_results
participant Builder as _build_scalar_value
participant Value as ScalarMetricValue
Bundle->>Walker: iterate metadata and value
Walker->>Builder: pass dimensions, value, attributes
Builder->>Builder: remove kind from dimensions
Builder->>Value: construct with kind and remaining dimensions
Value-->>Walker: yielded scalar value
Possibly related PRs
Suggested labels: review_needed_senior_swe, review_depth_deep Suggested reviewers: None identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
lewisjared
force-pushed
the
feat/layer-a-pmp
branch
from
674519b to
1d681d6
Compare
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/regression/diagnostic.json (1)
50-80: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winScrub local provenance from these regression fixtures
packages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/regression/diagnostic.jsonstill embeds/Users/jared/...,osx-64, andDarwinin the provenance block, and the same pattern appears in the matchingoutput.jsonsnapshots. Replace those fields with placeholders or sanitised values before committing so the golden data stays machine-agnostic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 37a71f41-75fa-46dd-9725-282a969d85df
📒 Files selected for processing (54)
changelog/775.feature.mdpackages/climate-ref-core/src/climate_ref_core/pycmec/metric.pypackages/climate-ref-core/tests/unit/pycmec/test_cmec_metric.pypackages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/annual_cycle.pypackages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/enso.pypackages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/variability_modes.pypackages/climate-ref-pmp/src/climate_ref_pmp/drivers/enso_driver.pypackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-pr/manifest.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-pr/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-ts/manifest.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-ts/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip7-ts/manifest.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip7-ts/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/regression/output.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip7/regression/output.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip6/regression/output.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip7/regression/output.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/unit/test_variability_modes.py
Comment on lines
+5
to
+7
| ENSO references that the metrics package scores against two observation datasets jointly | ||
| (for example `HadISST_GPCP-Monthly-3-2`) are kept as a single combined `reference_source_id` value, | ||
| because the score is one value that depends on both references and cannot be split per reference. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Minor grammar nit: unnecessary comma before "because".
Static analysis flags a restrictive "because" clause preceded by a comma.
✏️ Proposed fix
-ENSO references that the metrics package scores against two observation datasets jointly
-(for example `HadISST_GPCP-Monthly-3-2`) are kept as a single combined `reference_source_id` value,
-because the score is one value that depends on both references and cannot be split per reference.
+ENSO references that the metrics package scores against two observation datasets jointly
+(for example `HadISST_GPCP-Monthly-3-2`) are kept as a single combined `reference_source_id` value
+because the score is one value that depends on both references and cannot be split per reference.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ENSO references that the metrics package scores against two observation datasets jointly | |
| (for example `HadISST_GPCP-Monthly-3-2`) are kept as a single combined `reference_source_id` value, | |
| because the score is one value that depends on both references and cannot be split per reference. | |
| ENSO references that the metrics package scores against two observation datasets jointly | |
| (for example `HadISST_GPCP-Monthly-3-2`) are kept as a single combined `reference_source_id` value | |
| because the score is one value that depends on both references and cannot be split per reference. |
🧰 Tools
🪛 LanguageTool
[formatting] ~6-~6: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...gle combined reference_source_id value, because the score is one value that depends on ...
(COMMA_BEFORE_BECAUSE)
Source: Linters/SAST tools
lewisjared
force-pushed
the
feat/layer-a-pmp
branch
from
1d681d6 to
6f6a387
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/climate-ref-core/tests/unit/regression/test_capture.py (1)
170-243: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIntegration test doesn't exercise
_redact_source_pathsend-to-end.
_leaky_provenance'scommandLineonly contains{software_root}/conda/...paths — nopackages/climate-ref-<pkg>/segment — sotest_write_committed_bundle_redacts_and_placeholders_provenancenever triggers the new checkout-root redaction through the realwrite_committed_bundlepipeline. The new behaviour is only verified via the standalone_redact_source_pathsunit tests, not via the full sanitise → canonicalise pipeline interaction this PR relies on for the 21 re-minted PMP baselines.Consider adding a package-path segment (e.g.
.../packages/climate-ref-pmp/src/.../enso_driver.py) to thecommandLinefixture and asserting<SOURCE_DIR>appears in the committed output, to cover the interaction betweenPlaceholderMap.sanitise()ordering and_redact_source_paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 665cb9b9-c48d-4bb8-810b-900c8b8d4e65
📒 Files selected for processing (60)
changelog/775.feature.mdchangelog/775.fix.mdpackages/climate-ref-core/src/climate_ref_core/pycmec/metric.pypackages/climate-ref-core/src/climate_ref_core/regression/capture.pypackages/climate-ref-core/tests/unit/pycmec/test_cmec_metric.pypackages/climate-ref-core/tests/unit/regression/test_capture.pypackages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/annual_cycle.pypackages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/enso.pypackages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/variability_modes.pypackages/climate-ref-pmp/src/climate_ref_pmp/drivers/enso_driver.pypackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-pr/manifest.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-pr/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-pr/regression/output.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-ts/manifest.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-ts/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-ts/regression/output.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip7-ts/manifest.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip7-ts/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip7-ts/regression/output.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/regression/output.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip7/regression/output.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip6/regression/output.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip7/regression/output.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip6/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip7/regression/diagnostic.jsonpackages/climate-ref-pmp/tests/unit/test_variability_modes.py
✅ Files skipped from review due to trivial changes (17)
- changelog/775.fix.md
- packages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-pr/manifest.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip6/manifest.json
- packages/climate-ref-pmp/tests/test-data/annual-cycle/cmip7-ts/manifest.json
- packages/climate-ref-pmp/tests/test-data/enso_proc/cmip7/regression/output.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip7/manifest.json
- packages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-ts/manifest.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip7/manifest.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip6/manifest.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip7/manifest.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip7/manifest.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip6/manifest.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip7/manifest.json
- packages/climate-ref-pmp/tests/test-data/enso_tel/cmip7/regression/output.json
- packages/climate-ref-pmp/tests/test-data/enso_tel/cmip6/manifest.json
- packages/climate-ref-pmp/tests/test-data/enso_proc/cmip7/manifest.json
- packages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/manifest.json
🚧 Files skipped from review as they are similar to previous changes (32)
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip6/manifest.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip6/manifest.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip7/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/regression/output.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip6/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip6/manifest.json
- packages/climate-ref-core/src/climate_ref_core/pycmec/metric.py
- packages/climate-ref-pmp/tests/test-data/enso_tel/cmip6/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/enso_tel/cmip7/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip7/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip6/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip7/manifest.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip7/regression/diagnostic.json
- packages/climate-ref-pmp/src/climate_ref_pmp/drivers/enso_driver.py
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip6/manifest.json
- packages/climate-ref-pmp/tests/unit/test_variability_modes.py
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip7/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip6/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/regression/diagnostic.json
- packages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/annual_cycle.py
- packages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-ts/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip7/regression/diagnostic.json
- packages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/enso.py
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip7/manifest.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip6/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip6/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip6/regression/diagnostic.json
- packages/climate-ref-core/tests/unit/pycmec/test_cmec_metric.py
- packages/climate-ref-pmp/tests/test-data/enso_proc/cmip7/regression/diagnostic.json
- packages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip7/regression/diagnostic.json
- packages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/variability_modes.py
- packages/climate-ref-pmp/tests/test-data/enso_tel/cmip7/manifest.json
Unify PMP reference identity onto the shared metric-value contract. ENSO now keys reference identity on reference_source_id (previously reference_datasets), matching variability-modes and annual-cycle, and every PMP scalar declares its role with the first-class kind of "model". Core: iter_results lifts a kind bundle dimension into ScalarMetricValue.kind so a producer can declare a value's role inline; bundles without the dimension keep the default, so baselines and providers that predate the contract are unaffected. ENSO references scored jointly against two observation datasets (for example HadISST_GPCP-Monthly-3-2) are kept as a single combined reference_source_id value, because the score is one value that depends on both references and cannot be split per reference. Regression capture now redacts the source-checkout path and host OS in command-line provenance to portable <SOURCE_DIR> / <OS> placeholders, so committed baselines stay reproducible across platforms and checkouts. PMP regression baselines re-minted and Diagnostic.version bumped accordingly.
lewisjared
force-pushed
the
feat/layer-a-pmp
branch
from
6f6a387 to
a2fb09e
Compare
3 tasks
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
1 participant
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.
Description
Unifies PMP reference identity onto the shared metric-value contract. ENSO previously keyed reference identity on
reference_datasetswhile variability-modes and annual-cycle already usedreference_source_id; this makes every PMP diagnostic key reference identity the same way and declare its role with the first-classkind(modelfor all PMP scalars, which are model-vs-reference scores).Core mechanism:
iter_resultsnow lifts akindbundle dimension intoScalarMetricValue.kind, so a producer can declare a value's role inline while it stays a first-class field with its own column. Bundles that do not carry akinddimension keep the field default, so existing baselines and providers that predate the contract are unaffected.Combined references (the STOP-gated step in the plan): ENSO scores some metrics jointly against two observation datasets (for example
HadISST_GPCP-Monthly-3-2). These are kept as a single combinedreference_source_idvalue rather than split, because the score is one value that depends on both references and the source data does not expose a per-reference decomposition; splitting is left as a possible follow-up.The resulting ENSO
json_structureis["kind", "mip_id", "source_id", "member_id", "grid_label", "experiment_id", "metric", "reference_source_id"].All 21 PMP regression baselines were re-minted (17 via replay for the key/kind change; the 4 ENSO cases execute-minted) and
Diagnostic.versionbumped accordingly. The coupling gate routes all 21 cases toreplaywith zero failures.Portable regression provenance
While re-minting, the committed baselines were found to embed machine-specific values in the CMEC command-line provenance: the source-checkout path of in-repo driver/param scripts (e.g.
/home/jared/.../packages/climate-ref-pmp/.../enso_driver.py) and the hostOS(Darwin/Linux). These were pre-existing onmainand would drift between a macOS mint and a Linux CI execute. Regression capture now redacts both to portable<SOURCE_DIR>and<OS>placeholders (the source-path redaction is checkout-agnostic, so a Linux-minted and a macOS-re-derived baseline reduce to identical bytes). Applied symmetrically through the capture path, so replay reproduces the committed bundle.Surfaced while minting locally (not fixed here): #773 — on macOS the
Application Supportspace in the default software path breaks the PMP conda console-script shebang; the PMP conda-lock is also not resolved forosx-arm64, so local mints useMAMBA_PLATFORM=osx-64with a space-freeREF_SOFTWARE_ROOT.Checklist
Please confirm that this pull request has done the following:
changelog/Summary by CodeRabbit
kindfield and include consistent “model” role metadata.reference_source_id(including consistent handling when scoring is computed jointly).kind/modelstructure.