fix: update Stuart-Landau runtime#1362
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90b6c79974
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| int_steps: int = _DEFAULT_INT_STEPS, | ||
| dt: float = _DEFAULT_DT, | ||
| seed: int = 42, | ||
| fail_closed: bool = False, |
There was a problem hiding this comment.
Propagate fail-closed mode to evidence callers
When the OOS evidence generator calls rolling_es_proximity at benchmarks/rolling_es_proximity_oos.py:185, it omits this new argument, so the default remains the exploratory behavior that converts every per-window ValueError/FloatingPointError into NaN before writing/scoring the artifact. In that evidence context, contract failures are still hidden unless the benchmark is updated to pass fail_closed=True or the default is made fail-closed for evidence paths.
Useful? React with 👍 / 👎.
| f"INV-SL contract VIOLATED: K_low<K_high required, " | ||
| f"got K_low={K_low}, K_high={K_high}. Source: {source}." | ||
| ) | ||
| if not isinstance(K_steps, int) or K_steps < 2: |
There was a problem hiding this comment.
Accept NumPy integer sweep parameters
This new strict isinstance(..., int) check rejects valid integral values produced by NumPy/pandas parameter grids, such as np.int64(6), even though the previous validation accepted them and range/linspace can use them after normal integer coercion. In experiment sweeps where K_steps or int_steps comes from np.arange/a DataFrame, fit_stuart_landau now fails before running with ValueError: integer K_steps≥2 required; use numbers.Integral or coerce safely instead.
Useful? React with 👍 / 👎.
Review verdict: REQUEST CHANGES — not mergeable as-isReviewed against issue #1358 + reproduced the failing required checks locally. What's correct (confirmed)
Blockers — required checks, each reproduced locally
Scope gap vs #1358
Validation layer (in progress)Running Recommendation: fix 1–3 (mechanical), decide P0-1 scope, re-run CI. The physics direction (fitted-state sweep, μ audit, fail-closed) is sound. |
Validation result —
|
| line | guard | gap |
|---|---|---|
| L138 | mu_clip_mass < 0 or not isfinite |
Or→And survives — negative-but-finite not rejected by a test |
| L189 | not isfinite(K_low) or not isfinite(K_high) |
single-side-non-finite rejection untested |
| L199 | not int(K_steps) or K_steps < 2 |
untested per-branch |
| L204 | not int(int_steps) or int_steps < 1 |
untested per-branch |
| L209 | not isfinite(dt) or dt <= 0 |
untested per-branch |
| L402 | not isfinite(z0.real) or not isfinite(z0.imag) |
the NEW z0 fail-closed guard this PR adds is under-tested — Or→And survives (one component inf, other finite, not exercised) |
| L596 | es_proximity > es_threshold |
explosive-classification boundary not pinned |
Per the work-axis, these 7 become required follow-ups before this claim-critical module merges. Most important: L402 — the fail-closed INV-SL guard you added needs a test that feeds z0 with one non-finite component and asserts it raises (currently Or→And is behaviourally invisible to the suite). Add per-component boundary cases for the validation guards, then the kill-rate on the logic operators goes to 100% (the constant survivors are acceptable for physics).
Probing the sibling claim-critical modules next: core/cross_asset_kuramoto/invariants.py, core/kuramoto/second_order.py.
|
Debt-closure status for #1358 / #1362: Runtime/code debt resolved on latest head Green evidence on the same head:
PR Gate breakdown:
No failing pytest node id, lint error, type error, security finding, physics violation, or artifact was produced by the cancelled PR Gate run. This remaining debt is tracked separately as #1363: PR Gate fast-shard cancellation without failing test evidence. Verdict: #1362 runtime fix is validated by specialized gates; aggregate merge readiness is blocked only by cancelled PR Gate execution lane, not by an observed runtime-code failure. |
- python-quality: black-format core/physics/stuart_landau_es.py and scripts/check_invariant_registry_schema.py (left black-dirty by concurrent edits). - clamp registry realigned to post-black line numbers (193/198/354/404). - component-test-router: map .github/workflows/pr-gate.yml to artifact_freshness component so the secrets-exclude edit is a routed (not unmapped-required) surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Updates Stuart-Landau runtime handling and adds regression tests for issue 1358.