Add return_cc option to run_pipeline - #32
Merged
Merged
Conversation
run_pipeline(..., return_cc=True) returns (dvv, valid, cc) with the per-epoch stretching correlation coefficient, the input needed by uq_measurement.weaver_stretching_error. CC is collected for the fixed and moving references with the stretching estimator (the moving loop was already computing it and discarding it); NaN for other estimators and the inversion reference. Behavior-preserving: the default two-tuple return is unchanged, and CC-gating stays fixed-reference-only (the moving-reference CC is returned for error modelling but does not alter the valid mask). Verified against the golden expected-metrics suite (22 passed). Motivation: the noisepy-dvv-cloud pipeline needs the real per-epoch CC to replace a placeholder in its dvv_err_within (Weaver/Clarke) column. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR extends the codameter.deviations.run_pipeline API to optionally return the per-epoch stretching correlation coefficient (CC) alongside dvv and valid, enabling coherence-based error models to use real CC values while keeping the default return shape unchanged.
Changes:
- Add
return_ccoption torun_pipeline, returning(dvv, valid, cc)when enabled and preserving(dvv, valid)by default. - Collect per-epoch CC for moving-reference stretching via
_moving_reference(..., collect_cc=True); return NaN CC for non-stretching estimators and inversion reference. - Add targeted tests covering backward-compatible returns, CC behavior for fixed/moving stretching, and NaN CC for non-stretching configurations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/codameter/deviations.py |
Adds return_cc behavior and CC collection for moving-reference stretching while keeping fixed-only CC gating behavior. |
tests/test_deviations.py |
Adds TestReturnCC to verify return shapes, CC behavior, and backward compatibility. |
CHANGELOG.md |
Documents the new return_cc option and its behavior in the Unreleased section. |
.claude/settings.json |
Adds Claude permissions configuration (not directly related to the run_pipeline change). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+8
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Skill(deep-research)", | ||
| "Skill(deep-research:*)" | ||
| ] | ||
| } | ||
| } |
Per Copilot review: this file grants Skill(deep-research) permissions, unrelated to run_pipeline's return_cc option -- it isn't tracked on master and doesn't belong bundled into this change. Removing it here; whether it should exist in the repo at all is a separate decision.
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.
What
run_pipeline(ccfs, t, fs, cfg, eps_max=..., return_cc=True)now returns(dvv, valid, cc), whereccis the per-epoch stretching correlation coefficient. The default call is unchanged and still returns(dvv, valid).measure_stretching(as already computed)._moving_reference(collect_cc=True)).Why
Coherence-based error models need the per-epoch CC — specifically
uq_measurement.weaver_stretching_error, which converts CC into the Weaver/Clarke (2011) aleatoric floor. The new noisepy-dvv-cloud pipeline (single-station dv/v on AWS Batch) currently fakes CC=0.8 for itsdvv_err_withinParquet column; this PR lets it use the real value.Behavior safety
ref == "fixed"in the gate condition. The moving-reference CC is returned for error modelling but does not altervalid.tests/test_deviations.pygains aTestReturnCCclass (4 tests): default two-tuple preserved, dvv/valid identical with and withoutreturn_cc, moving warm-up NaNs, non-stretching NaN CC.test_deviations.py(8 passed) and the golden expected-metrics suitetest_golden.py(22 passed) run clean on Python 3.10.🤖 Generated with Claude Code