Skip to content

test(inference): prevent false zero uncertainty after Rust migration - #722

Closed
seonghobae wants to merge 1 commit into
mainfrom
test/inference-nonfinite-se-red-625
Closed

test(inference): prevent false zero uncertainty after Rust migration#722
seonghobae wants to merge 1 commit into
mainfrom
test/inference-nonfinite-se-red-625

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Post-integration scientific correction for #625

Protected main at branch creation and final pre-PR refetch: 557a065350abcfb963d5d0a010e2ab79b7e9ff47, which contains merged PR #647.

Fresh protected-main inspection found a consequential parity defect in the just-integrated Rust covariance/SE slice. mlsirm_core::inference::standard_errors_from_vcov() currently converts every diagonal that is not both finite and positive to 0.0. The protected Python contract before migration was sqrt(max(diag, 0)), under which finite negative values clamp to zero, but NaN remains NaN and +Inf remains +Inf. The current Rust behavior therefore turns undefined/infinite uncertainty into exact zero standard error—false precision.

vcov_from_hessian() also lacks a finite-entry preflight, so NaN/Inf observed-information inputs can enter inversion/eigendecomposition instead of failing through a governed error surface.

This Draft adds only the smallest intentional REDs:

  • finite positive/zero/negative covariance diagonals retain the existing 2.0/0.0/0.0 semantics;
  • NaN diagonal uncertainty remains NaN and +Inf remains +Inf, both through the public wrapper and direct Rust binding; and
  • NaN/+Inf/-Inf Hessian inputs fail closed with a stable finite-Hessian error rather than producing an uncontrolled covariance artifact.

Current protected main is expected to fail because Rust returns zero for the non-finite SE cases and has no finite Hessian preflight. Setup/import/fixture failure is not valid RED.

Minimum GREEN

  1. Preserve exact pre-migration SE semantics for finite values and non-finite uncertainty: finite negative→0, NaN→NaN, +Inf→+Inf. If a fail-closed non-finite covariance policy is scientifically preferred, make that a separately test-locked/publicly documented decision; never coerce invalid/infinite uncertainty to zero.
  2. Add Rust finite-entry preflight for observed-information/Hessian covariance inversion so non-finite matrices fail with one stable non-reflective error before inversion/pseudoinverse work.
  3. Preserve inverse→Moore–Penrose fallback, symmetry, rcond, ordinary singular/near-singular behavior and caller immutability.
  4. Add direct Rust↔public parity/property tests over finite positive/zero/negative/NaN/+Inf covariance diagonals and non-finite Hessian cases. Ensure downstream uncertainty/report paths cannot interpret invalid uncertainty as precise certainty.
  5. Add concise APA 7 doctoring/changelog only after GREEN, then run all same-head Python/Rust/PyO3/package/GPU/fuzz/Security/SAST/current-head review gates.
  6. Continue architecture: migrate observed-information and covariance numerics from Python to Rust #625 afterward for observed-information finite-difference and second-order eigensystem Rust ownership; this correction alone does not close the umbrella issue.

Do not widen into observed-information Hessian construction, second-order eigenanalysis, dependencies/workflows/version/release, or canonical #604 docs. Keep Draft through RED→GREEN. Advances #625.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5bf15a11-7d94-4f33-a1cc-894600bf6e11

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Exact bounded implementation handoff for Draft #722. Before any write, refetch source head 7869c3150846366a3712915f8692ccfcea993ec0 and protected main 20b029a237e7819b5a4f7d74a1b2bbfb1b6755f7; abort this branch if either moved or another source writer appeared. Preserve the intentional RED history and reconcile current main non-destructively before final acceptance.

Exact CI run 31478837656 reached the intended scientific boundary on both Python 3.12 and 3.14. Package/reinstall/release acceptance, Rust/PyO3, explicit GPU smoke, fuzz, Security Scan and SAST are green. Python has exactly five intended failures in tests/test_inference_nonfinite_uncertainty.py: Rust currently maps NaN/+Inf covariance diagonals to zero standard error, and vcov_from_hessian accepts NaN/+Inf/-Inf Hessian entries instead of failing closed. The failures are production-boundary REDs, not setup/import failures.

Implement the minimum GREEN only:

  1. In the Rust inference owner, preserve the pre-migration sqrt(max(diag, 0)) semantics for finite covariance diagonals while preserving undefined/infinite uncertainty rather than converting it to certainty: finite positive→sqrt, finite zero/negative→0, NaN→NaN, +Inf→+Inf. Do not silently coerce non-finite uncertainty to zero.
  2. Add a Rust finite-entry preflight for the Hessian/observed-information covariance path before inverse/pseudoinverse/eigensystem work. Map failure through PyO3/public Python to one stable non-reflective ValueError whose message satisfies hessian.*finite; do not echo matrix content.
  3. Preserve inverse→Moore–Penrose fallback, symmetry, rcond, ordinary singular/near-singular behavior, caller immutability and all accepted refactor(inference): move covariance arithmetic to Rust #647 ownership semantics.
  4. Add focused Rust tests for finite/non-finite SE mapping and Hessian preflight if missing; retain public/direct-Rust parity tests. Do not widen into observed-information construction, second-order eigenanalysis, dependencies, workflows, GPU, model formulas or canonical docs: establish canonical product and architecture baseline #604 docs.
  5. After GREEN, add only concise method doctoring/changelog if required by repository policy, grounded in authoritative numerical/statistical references already governing this inference boundary. Run focused inference tests, Rust workspace/PyO3, changelog check and git diff --check; then leave Draft for fresh full exact-head CI/security/review.

This correction must prevent false precision. Invalid or infinite uncertainty must never become an exact zero-SE signal.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Superseded by GREEN #725 (preserve non-finite SE; reject non-finite Hessians).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant