Lock SDiD synthetic trajectory under joint reordering of controls and weights - #1210
Merged
drbenvincent merged 3 commits intoSep 26, 2026
Merged
Conversation
… weights. The einsum formula is invariant to that permutation, and no test asserted it. Co-authored-by: Cursor <cursoragent@cursor.com>
drbenvincent
commented
Sep 26, 2026
drbenvincent
left a comment
drbenvincent
left a comment
Collaborator
Author
There was a problem hiding this comment.
Review: PR 1210 vs issue #1203
Verdict: APPROVE (posted as a comment because GitHub disallows self-approval on this PR).
Definition of done (issue #1203)
- Add a regression test that a joint permutation of control rows and the
omegaunit axis leavessc_allunchanged. - Do not change
_compute_synthetic_and_gaps.
Assessment
This PR meets the definition of done.
Evidence
- Diff touches only
causalpy/tests/test_sdid_helpers.py(+21). Production helper is untouched. - New test
TestComputeSyntheticAndGaps::test_joint_reordering_of_controls_and_weights_is_invariantmatches the issue probe:omegashape(2, 4, 5), fixed perm[3, 0, 4, 1, 2], joint reorder viaomega[..., perm]andY_co[perm], assert onsc_all. - Locally:
pytest causalpy/tests/test_sdid_helpers.py::TestComputeSyntheticAndGaps --no-cov→ 2 passed.
Findings by file
No correctness, contract, or DoD gaps found.
Residual notes (non-blocking)
atol=1e-12is looser than the issue’s observed ~1e-15 max abs diff; still appropriate for float invariance and not flaky.- Gaps are not asserted; that is fine because #1203 only requires
sc_allinvariance. pre-commit.ci - prreported failure while the repoprekcheck succeeded; unrelated to this test-only change.
Verdict
APPROVE — regression lock for the claimed invariance is in place without modifying _compute_synthetic_and_gaps.
drbenvincent
marked this pull request as ready for review
September 26, 2026 14:31
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## pymc6_and_pymcmarketing1_migration #1210 +/- ##
===================================================================
Coverage 97.14% 97.15%
===================================================================
Files 130 130
Lines 24033 24044 +11
Branches 1349 1349
===================================================================
+ Hits 23348 23359 +11
Misses 477 477
Partials 208 208 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
drbenvincent
added
tests
daimon-pymclabs
changed the base branch from
main
to
pymc6_and_pymcmarketing1_migration
September 26, 2026 16:01
…der-controls Bring the branch up to date after #1214 synced main into the migration branch.
drbenvincent
merged commit Sep 26, 2026
f8fa199
into
pymc6_and_pymcmarketing1_migration
16 checks passed
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.
Closes #1203
Summary
_compute_synthetic_and_gapsunchanged.Test plan
pytest causalpy/tests/test_sdid_helpers.py::TestComputeSyntheticAndGaps --no-covMade with Cursor