Skip to content

feat(core): standardize covariance through Rust owner contract - #1722

Open
seonghobae wants to merge 21 commits into
mainfrom
fix/covariance-standardization-1720
Open

feat(core): standardize covariance through Rust owner contract#1722
seonghobae wants to merge 21 commits into
mainfrom
fix/covariance-standardization-1720

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Closes #1720.

Scope

Move reusable static covariance-to-correlation standardization into the canonical Rust numerical owner instead of allowing TEPP temporal/event code to own duplicate arithmetic.

  • add mlsirm_core::covariance_standardization as a domain-neutral Rust module;
  • expose versioned contract fast_mlsirm.covariance_standardization@1.0.0;
  • evaluate the scalar positive-variance specialization through arithmetic rather than a hard-coded constant;
  • fail closed on zero/negative/non-finite variances, malformed shapes, non-finite matrix evidence, asymmetric covariance evidence, and exactly inadmissible pairwise covariance;
  • provide the matrix-compatible D^{-1/2} Sigma D^{-1/2} contract without ctsem names, clocks, EventTime admission, or TEPP temporal-state semantics;
  • preserve deterministic CPU f64, scale invariance, variable-order invariance, and typed numerical failure evidence.

TEPP remains the owner of temporal/event semantics and may consume this only after an immutable released/versioned fast-mlsirm contract exists.

TDD / concurrent-writer convergence

Two same-issue implementations appeared from the same protected-main base. #1721 was closed unmerged only after its unique public-contract tests were preserved here; its undocumented 64*EPSILON / 128*EPSILON admission tolerances were intentionally not carried forward.

  • RED 69ae2c40c4a459f9e946f9dc0850019dde06b25a proved that heuristic approximate symmetry and silent acceptance/clamping of an actually invalid covariance were not acceptable.
  • GREEN 6ed74a7d37492976133bd5450efcf0c1d6ecaa80 removed those heuristics and required exact mirrored binary64 inputs.
  • Public-contract evidence through 13d2159ae15903b26b5f002c9caa583db5b110b1 preserves the version identity, scalar scale invariance/fail-closed behavior, known matrix recovery, scale invariance, malformed-input admission, and invalid-covariance rejection that were unique to feat(core): own reusable covariance standardization #1721.
  • RED 82d4362082ac50e8987f96b731126b8e68896d8e reproduced an exactly admissible represented covariance for which sequential binary64 division rounds the raw result to next_up(1.0).
  • GREEN e7a9ebf1a35162af60b30c836965fa5fc0afa6c5 replaced the floating threshold with an exact represented-input check of c² <= v_i*v_j using integer significands and powers of two. Only after that exact admission proof may boundary arithmetic be projected back to [-1,1]; genuinely invalid covariance still fails closed.
  • RED a15fba524c74ac336827068afe610f3818d8a3aa reproduced order-dependent intermediate underflow with f64::MAX, the smallest positive subnormal variance, and a nonzero admissible covariance.
  • GREEN 6bda7010f2c9796037544446e26b591f2d4980fe divides by the smaller marginal standard deviation first, making that correlation nonzero and permutation-invariant while retaining the exact pairwise admission proof.
  • RED 759c9dbddcf78f5dc4a6877df0ffa457c290eae3 reproduced the remaining exact-binary64 symmetry defect: Rust numeric equality treated +0.0 and -0.0 as equal mirrored evidence even though the documented contract requires exact represented symmetry.
  • GREEN 73a930e76b084ea7eae08465bb0e72646fbdac6a compares mirrored cells by to_bits(), so signed-zero mismatch now fails closed without changing the already-reviewed pairwise-admission, roundoff, or scale-order behavior.
  • Exact-head follow-up repaired the scalar specialization after review proved (1/sqrt(v))*v*(1/sqrt(v)) can round to next_up(1.0) for finite positive inputs such as v=3. After finite-positive admission, the scalar contract now evaluates v/v, preserving an arithmetic implementation while returning exact binary64 1.0 for an admitted scalar variance instead of transferring matrix-rounding artifacts into the one-variable correlation contract.
  • Non-destructive reconciliation f70cb9beffdacca30ded281c804ed80504baa47e integrates protected main@493326f2de49ea1704da0ded19868ed05d2fe00f as the second parent while preserving this PR's Rust owner-contract delta. Commit 28b0305595107fd0ba21d7b27c1ac5db68ae8bf1 adds the required unreleased changelog fragment. The writer ref advanced with force=false; relative to protected main it is behind_by=0 and the effective scope is the original four files plus docs/changelog.d/1722-covariance-standardization.md.

All currently observed review threads on the predecessor source head were resolved after the corresponding source/test proof. No predecessor-head check, review, or artifact is transferred to the reconciled head.

Scientific / DDD boundary

Driver, Oud, & Voelkle (2017), DOI 10.18637/jss.v077.i05, supplies the motivating ctsem use case. The owner kernel implements the ordinary covariance-to-correlation identity and does not claim that pairwise admissibility proves full PSD. PSD, model identification, EventTime admission, and temporal state semantics remain separate caller/model invariants.

This PR adds Rust numerical ownership only. It adds no Python production arithmetic, provider/LLM path, temporal event ontology, database coupling, or cross-service SQL. context-graph-contracts and EA Core remain read-only dependencies under their dedicated writer; no unreleased sibling PR is treated as a production contract.

Landing

Current exact source head: 28b0305595107fd0ba21d7b27c1ac5db68ae8bf1.
Protected base observed at this update: main@493326f2de49ea1704da0ded19868ed05d2fe00f.

Fresh exact-current GitHub Actions evidence and a qualifying independent approval are required after this reconciliation. The live repository default-branch ruleset requires one qualifying approval, dismisses stale reviews on push, requires review-thread resolution, requires approval from someone other than the last pusher, and requires extra approval for unattributed changes. Queued/pending/predecessor evidence is non-passing. No self-approval, routine admin bypass, force-push, destructive rebase, or gate weakening is authorized.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a versioned mlsirm-core covariance-to-correlation kernel. The implementation provides scalar and matrix APIs, exact binary64 validation, typed fail-closed errors, sequential normalization, result clamping, contract tests, and owner documentation.

Changes

Covariance Standardization

Layer / File(s) Summary
Define and expose the standardization contract
crates/mlsirm-core/src/covariance_standardization.rs, crates/mlsirm-core/src/entrypoint.rs, docs/papers/covariance-standardization-owner-contract.md
Defines the standardization identity, versioned public contract, typed errors, module export, and owner contract.
Implement exact numeric validation and scalar standardization
crates/mlsirm-core/src/covariance_standardization.rs
Adds exact binary64 comparison helpers, pairwise admissibility checks, scalar variance standardization, and unit coverage.
Standardize covariance matrices and validate behavior
crates/mlsirm-core/src/covariance_standardization.rs, crates/mlsirm-core/tests/covariance_standardization_contract.rs
Adds matrix shape, finiteness, variance, symmetry, and pairwise validation. It standardizes matrix entries sequentially, clamps certified results, and tests valid and invalid cases.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟡 Moderate · up to e8278

The new covariance standardization API can currently accept signed-zero asymmetric covariance cells and can produce an incorrect zero result for some valid extreme-valued inputs. These localized correctness issues should be fixed before the PR is merged.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant standardize_covariance_matrix
  participant pairwise_covariance_is_admissible
  Caller->>standardize_covariance_matrix: covariance and dimension
  standardize_covariance_matrix->>pairwise_covariance_is_admissible: covariance and diagonal variances
  pairwise_covariance_is_admissible-->>standardize_covariance_matrix: admissibility result
  standardize_covariance_matrix-->>Caller: correlation matrix or typed error
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR satisfies the coding objectives in [#1720]. It adds a formulation-neutral scalar and matrix covariance-standardization kernel, validates invalid inputs, exposes a versioned contract, preserves …
Out of Scope Changes check ✅ Passed The code, tests, module exposure, and owner-contract documentation directly support the covariance-standardization kernel required by [#1720]. No unrelated production changes are present.
Docstring Coverage ✅ Passed Docstring coverage is 86.36% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 3 files. (1 skipped: 1 …
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding covariance standardization to the Rust core contract.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/covariance-standardization-1720

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae seonghobae added priority: high status: needs-review Open pull request requiring current-head review or checks type: feature New or expanded product capability labels Sep 2, 2026 — with ChatGPT Codex Connector
Take current protected main as the authoritative tree and preserve only #1722's four-file Rust covariance-standardization owner-contract delta. Keep the newly merged binary-response measurement contract and both histories without force push or destructive rebase.

Copy link
Copy Markdown
Contributor Author

Fresh exact-head landing evidence for unchanged 338dbb2d25f32b0e201102e7bf73076846fb57b3 on protected main@b5a3a0c1057d4b53d7a4bb18e0de69f630c2b45c:

  • CI 33631932777 has real source execution GREEN on package (100352858994), Python 3.12 (100352859146), fuzz (100352859194), Rust workspace/PyO3 (100352859297), Python 3.14 (100352859344), and GPU-smoke (100352859442). Those jobs acquired runners, checked out this exact SHA, and completed successfully.
  • The same CI run is still non-passing because aggregate job 100553448367 (python) remains queued with no steps.
  • Required CodeQL PR 33631934624 is terminal startup_failure with jobs=[].
  • OSV 33631933401, SAST Semgrep 33631932758, repository CodeQL 33631932757, Scorecard 33631932688, Security Scan 33631932733, and ClusterFuzzLite 33631932736 are terminal success on the same head.
  • All currently returned inline review threads are resolved, but there is still no qualifying independent APPROVED review.

The aggregate-queue / CodeQL pre-materialization split is handed to canonical central owner .github#712 with these exact run/job IDs. No source churn, predecessor transfer, bypass, or gate weakening is warranted. Normal merge remains blocked until the unchanged head has terminal success for every applicable required workflow plus the live ruleset's qualifying independent approval.

Copy link
Copy Markdown
Contributor Author

Current-main reconciliation and release-traceability repair published.

  • exact head: 28b0305595107fd0ba21d7b27c1ac5db68ae8bf1
  • protected base: main@493326f2de49ea1704da0ded19868ed05d2fe00f
  • merge commit: f70cb9beffdacca30ded281c804ed80504baa47e with parents 338dbb2d25f32b0e201102e7bf73076846fb57b3 and protected main
  • compare: ahead 21 / behind 0; original four Rust owner-contract paths preserved and one docs/changelog.d/1722-covariance-standardization.md fragment added
  • exact-head runs newly admitted: CI 34076849582, ClusterFuzzLite 34076849579, CodeQL 34076849581, required CodeQL PR 34076849542, Security 34076849578, Semgrep 34076849554; all currently queued and none is represented as GREEN

The local environment has no cargo executable, so predecessor Rust GREEN was not transferred or restated as current-head proof. The changelog fragment parsed successfully; the aggregate renderer correctly reports that CHANGELOG.md remains release-cut output rather than a feature-branch rewrite.

The writer ref advanced with force=false. No destructive rebase, self-approval, bypass, synthetic status, empty push, or manual rerun was used.

Copy link
Copy Markdown
Contributor Author

Fresh exact-head evidence update for 28b0305595107fd0ba21d7b27c1ac5db68ae8bf1: repository CI 34076849582 and native CodeQL 34076849581 are now terminal success. SAST Semgrep 34076849554, CodeQL PR 34076849542, ClusterFuzzLite 34076849579, and Security Scan 34076849578 remain queued. The head/base remain unchanged, direct main@493326f2de49ea1704da0ded19868ed05d2fe00f, mergeable, behind 0. These two GREEN checks do not supply the remaining security/fuzz/review/approval evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request priority: high status: needs-review Open pull request requiring current-head review or checks type: feature New or expanded product capability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Canonical owner] Add static covariance-standardization kernel for TEPP TIPREDVARstd migration

1 participant