spec(dqn): SP1 numerical stability — revise per critical review

9 substantive issues addressed inline:

1. ISV-driven design elevated from 'if applicable' to MANDATORY for
   all dynamic bounds in SP1 fixes. Numerical-stability ε bounds are
   the only carve-out (Invariant 1). Hardcoded tuning constants for
   dynamic ranges explicitly rejected.

2. F0 Sharpe regression criterion changed from absolute (≥55) to
   ratio-based (≥95% of latest baseline; floor 53.08 currently).
   Prevents iterative erosion across multiple fix commits.

3. 'F1 trending positive' replaced with concrete monotone-improvement
   test: Best Sharpe at last epoch ≥ Best Sharpe at first epoch of
   the same fold.

4. Pass criterion distinguishes NaN-CLAMPED-TO-ZERO (failure) from
   'Genuine grad collapse' (legitimate observation, permitted) per
   the existing infrastructure from commit d1808df14.

5. Multi-source NaN scenarios explicitly supported — γ + β may
   identify multiple kernels; SP1 fixes ALL within the same cycle.

6. F0-safety paper-review gate added BEFORE smoke validation. Audit
   doc carries 'F0 risk' (low/medium/high) per proposed fix; high-
   risk fixes get math-on-paper inspection before consuming L40S.

7. Audit doc structure now requires 'ISV bound option' column —
   forces ISV-first thinking at audit stage, not as afterthought.

8. Anti-patterns expanded: micro-clamping (per-op clamping that hides
   upstream causes); combining unrelated fixes (anti-pattern of the
   rich-commit principle); hardcoded constants for dynamic bounds.

9. 48-slot allocation explicitly justified (24 used + 12 new + 12
   headroom) AND marked reviewable by SP2 if right-size differs.

All 5 design sections preserved structurally; revisions integrated
inline. Per brainstorming skill: spec self-review fixes applied
without re-review cycle.
This commit is contained in:
jgrusewski
2026-04-29 22:59:00 +02:00
parent dab5990287
commit 792812baa1

View File

@@ -25,16 +25,17 @@ SP1 → SP2 → SP3 build sequentially: SP1 reveals what guards were missing →
SP1 closes when ALL of:
1. The kernel/operation producing the first NaN at F1 ep2 is identified and fixed (per γ + β methodology below).
1. The kernel/operation(s) producing the first NaN at F1 ep2 is identified and fixed (per γ + β methodology below). **Multi-source case explicitly supported**: γ + β may identify multiple independent NaN-producing kernels; SP1 fixes ALL of them within the same investigation cycle.
2. L40S `multi_fold_convergence` smoke runs cleanly:
- All 3 folds train through 5 epochs (no early-stop from gradient collapse).
- F0 Best Sharpe ≥ 55 (no regression vs current 55.87 baseline).
- F1 Best Sharpe ≥ 0 and trending positive across epochs.
- F2 Best Sharpe ≥ 0 and trending positive across epochs.
- Zero `NaN-CLAMPED-TO-ZERO` log lines in the smoke output.
- F0 Best Sharpe ≥ 95% of the latest measured F0 baseline on the parent commit (currently 55.87 → floor 53.08). Ratio-based prevents accidental erosion of the working path across iterative fixes.
- F1 Best Sharpe at the LAST epoch ≥ Best Sharpe at the FIRST epoch of the same fold (concrete monotone-improvement test; Best Sharpe at epoch 5 ≥ epoch 1's Best Sharpe).
- F2 Best Sharpe at the LAST epoch ≥ Best Sharpe at the FIRST epoch of the same fold (same standard).
- Zero `NaN-CLAMPED-TO-ZERO` log lines in the smoke output (the NaN-derived collapse path).
- Zero `NaN/Inf at step` errors (training-guard `halt_nan` path).
- All 48 NaN flag slots remain at zero throughout the run.
3. Permanent diagnostic instrumentation is committed (per Section: Components below) — the 24 → 48 flag expansion stays as production-grade regression sentinel.
- "Genuine grad collapse" warn lines (legitimate near-zero gradients, no NaN flags set) are PERMITTED — they're observations, not failures, and indicate the diagnostic distinguishes real near-zero gradients from NaN-clamped-to-zero.
- All 48 NaN flag slots remain at zero throughout the run (verified by grep on smoke log).
3. Permanent diagnostic instrumentation is committed (per Section: Components below) — the 24 → 48 flag expansion stays as production-grade regression sentinel. Buffer size reviewable by SP2 framework codification.
---
@@ -65,11 +66,12 @@ SP1 is a **two-phase investigation with always-landing instrumentation, followed
### Phase C — Surgical Fix (combined / rich commit)
- The fix lands wherever γ + β jointly identify the source.
- The fix lands wherever γ + β jointly identify the source(s) — multi-source scenarios explicitly supported.
- Constraints:
- Principled (no quickfixes per `feedback_no_quickfixes`; ISV-driven if a bound is needed per `feedback_isv_for_adaptive_bounds`; additive guards inherit existing project patterns).
- F0 Best Sharpe ≥ 55 preserved (no regression on the working path).
- **Combined fix: when investigation reveals multiple related issues, they land together in a single rich commit per `feedback_no_partial_refactor`.** No artificial splitting into one-issue-per-commit.
- Principled (no quickfixes per `feedback_no_quickfixes`).
- **MANDATORY ISV-driven design**: every dynamic bound, threshold, clamp range, or floor introduced by SP1 MUST derive from an existing ISV slot or a new ISV slot if a new diagnostic is needed (per `feedback_isv_for_adaptive_bounds` + `feedback_adaptive_not_tuned`). The ONLY exception is numerical-stability ε bounds (Invariant 1 carve-out — e.g., `1e-6` divide-by-zero floor, `THOMPSON_MAX_ATOMS = 128` stack array sizing). Hardcoded "tuning" constants for dynamic ranges are explicitly rejected; if a knob has dynamic range that warrants adaptation, it MUST be ISV-driven. Existing ISV slots that fixes can leverage: `Q_ABS_REF_INDEX = 16`, `Q_DIR_ABS_REF_INDEX = 21`, `H_S2_RMS_EMA_INDEX`, `Q_DRIFT_RATE_INDEX = 129`, `FOLD_WARMUP_FACTOR_INDEX = 130`, plus any additions justified by SP1's specific findings.
- F0 Best Sharpe ≥ 95% of latest baseline (ratio-based, see Success Criterion).
- **Combined fix: when investigation reveals multiple RELATED issues, they land together in a single rich commit per `feedback_no_partial_refactor`.** No artificial splitting of related work. UNRELATED fixes (e.g., a kernel issue and an unrelated docs typo) MAY remain separate per single-concern commit hygiene.
- Companion: any new SAFE_DIV / SAFE_LOG / CLAMP_FINITE pattern introduced here gets documented in the audit doc for SP2's framework adoption.
### Phase D — Multi-fold Validation
@@ -92,13 +94,23 @@ SP1 is a **two-phase investigation with always-landing instrumentation, followed
### A. Backward NaN Audit Document *(Phase A output)*
- **Path:** `docs/dqn-backward-nan-audit.md`
- **Structure:** per-kernel sections covering name, file location, output buffers, accumulator targets, identified unsafe patterns (with line-level citations), severity (confirmed/suspect/none), proposed guard if any.
- **Structure:** per-kernel sections covering:
- Name + file location + line range
- Output buffers (what kernel writes)
- Accumulator targets (what kernel atomicAdd / saxpy into)
- Identified unsafe patterns (with line-level citations)
- Severity: confirmed / suspect / clean
- **Proposed guard form** (e.g., `isfinite()` + clamp, replace `sqrtf(x)` with `sqrtf(fmaxf(x, 0))`, etc.)
- **ISV bound option**: which existing ISV slot could parameterize the guard's bound, OR justification for why a new ISV slot is needed, OR justification for why this is a numerical-stability ε bound (Invariant 1 carve-out — only acceptable non-ISV form)
- **F0 risk assessment**: low / medium / high — does the proposed fix have any chance of changing F0 training dynamics? Used as paper-review gate before smoke validation.
- Cross-references `session_2026-04-05_nan_investigation.md` for known-residual context.
- Becomes durable artifact for SP2 (framework codification) and SP3 (structural-fix scoping).
### B. Expanded NaN Flag Buffer *(Phase B foundation)*
- `nan_flags_buf` grows 24 → 48 (24 already used: 0-12 + 13-15 reserved + 16-20 GRN + 21-23 reserved).
- Math: 24 used + 12 new (slots 24-35 per Components C below) + 12 reserved headroom = 48. The 12 headroom slots cover anticipated SP2 framework checker hooks + SP3 structural observers.
- The 48-slot size is **reviewable by SP2 framework codification** — if SP2 finds the right-size is 36 (no headroom waste) or 64 (more coverage), it may resize at that point. SP1's job is to expand to 48 with the new slots; SP2 may right-size.
- `read_nan_flags()` signature: `[i32; 48]`.
- Name table updated in BOTH consumer sites (`halt_nan` block + `halt_grad_collapse` block from commit `d1808df14`).
- Allocation site (`stream.alloc_zeros::<i32>(48)`).
@@ -220,7 +232,8 @@ The runtime NaN detection flow becomes permanent infrastructure. Every future ke
### Operating Principles
- **No deferrals.** Anomalies and side-issues uncovered during SP1 get investigated and fixed within SP1, not punted to SP2 or SP3.
- **Combined fixes — rich commits.** When investigation reveals multiple related fixes, they land together in a single coherent commit per `feedback_no_partial_refactor`. No artificial splitting into one-issue-per-commit.
- **Combined RELATED fixes — rich commits.** When investigation reveals multiple related fixes, they land together in a single coherent commit per `feedback_no_partial_refactor`. UNRELATED fixes (separate concerns) may stay in separate commits per single-concern commit hygiene.
- **Mandatory ISV-driven design for dynamic bounds.** Per `feedback_isv_for_adaptive_bounds` + `feedback_adaptive_not_tuned`: every dynamic bound, threshold, clamp, or floor in SP1's surgical fix MUST derive from an ISV slot (existing or new). Numerical-stability ε bounds (Invariant 1) are the only carve-out. Hardcoded "tuning" constants for dynamic ranges are explicitly rejected.
### Specific failure modes
@@ -241,9 +254,10 @@ The runtime NaN detection flow becomes permanent infrastructure. Every future ke
- Source-only fix that leaves downstream kernels still flagging on edge cases = incomplete; doesn't satisfy SP1 multi-fold criterion.
**(4) Fix introduces F0 regression**
- Hard constraint: F0 Best Sharpe ≥ 55.
- If a commit drops F0 below threshold: that commit is incorrect; revise within SP1, do not commit a known regression.
- Pre-flight check: every smoke run reports F0 Sharpe; commit only if F0 ≥ 55 AND F1+F2 satisfy criterion.
- Hard constraint: F0 Best Sharpe ≥ 95% of the latest F0 baseline (currently 55.87 → floor 53.08). Ratio-based prevents iterative erosion.
- **F0-safety paper review BEFORE smoke**: every proposed fix in the audit doc carries an "F0 risk" assessment (low/medium/high). High-risk fixes get extra scrutiny on paper before consuming an L40S smoke; low-risk fixes go straight to smoke. Paper review explicitly checks: does the proposed guard fire on F0's data distribution? If yes, what does F0's training look like with the guard active? (Answered by inspecting the guard's mathematical effect on F0-typical input ranges, before committing.)
- If a commit drops F0 below the ratio threshold: that commit is incorrect; revise within SP1, do not commit a known regression.
- Pre-flight check: every smoke run reports F0 Sharpe; commit only if F0 ≥ 95% baseline AND F1+F2 satisfy criterion.
**(5) Fix passes F1 but exposes F2 instability of a different class**
- **NO escalation to SP3.** Investigate F2's failure within SP1 using the same γ + β + fix loop.
@@ -267,10 +281,13 @@ The runtime NaN detection flow becomes permanent infrastructure. Every future ke
### Anti-Patterns Explicitly Rejected
- Globally clamping all backward outputs to a fixed range (masks bugs; violates `feedback_no_quickfixes`).
- **Micro-clamping individual operations without identifying upstream cause** (e.g., per-element `fminf(x, 1e6)` everywhere). Symptom-hiding disguised as defense; violates `feedback_no_quickfixes`. The fix must address WHY the value is extreme, not just clamp it.
- Disabling a loss component to make F1 train (violates `feedback_no_functionality_removal`).
- Increasing `gradient_collapse_patience` to "let training continue past NaN" (hides the issue).
- Splitting one logical fix across multiple commits to "make commits smaller" (fights `feedback_no_partial_refactor`).
- **Conversely: combining unrelated fixes** ("while we're here, let me also rename this variable") — keep rich commits focused on related concerns only.
- Punting any anomaly to SP2 or SP3 with "we'll get to it later" (prohibited per operating principles above).
- **Hardcoded constants for dynamic bounds** — every dynamic bound MUST be ISV-driven (mandatory operating principle). Hardcoded `epsilon = 0.05` for some clamp range, where 0.05 is "experimentally chosen", is rejected; that bound must derive from an ISV signal.
- Adding more `tracing::warn!` instead of NaN flags (warns get suppressed; flag-buffer write is captured-graph-safe).
---