fix(sp18-v2): restore-best precedes shrink-and-perturb at fold transition
`train_walk_forward` called `reset_for_fold().await?` directly at the fold boundary, which applies `shrink_and_perturb(α=0.8, σ=0.01)` to whatever `params_flat` holds. At end-of-fold, `params_flat` carries the LATEST-EPOCH decayed weights, NOT the best-Sharpe snapshot saved mid-fold. Fold N+1 then started from `0.8 × decayed + noise` and carried the within-fold edge-decay forward. Insert `restore_best_gpu_params()` BEFORE `reset_for_fold` so S&P operates on the best-Sharpe snapshot. Cold-start guard swallows the "no snapshot saved" error on the first fold (or any fold without a Sharpe improvement) — matches pre-fix behavior on cold start, no regression. State interaction with `reset_for_fold`: restore-best writes `params_flat` ONLY (online weights); reset_for_fold then runs S&P on the restored online, hard-syncs target ← restored online, and resets Adam state on every optimizer. Non-conflicting. `best_params_snapshot` is constructor-init `None` on FusedTrainingCtx and is NEVER cleared at fold boundary. The trainer's `self.best_sharpe` IS reset to NEG_INFINITY in `reset_for_fold` so the first improvement in fold N+1 will overwrite the snapshot. Until then, the snapshot holds whichever fold most recently saved a peak — intended training-scoped behavior. Strict within-fold semantics flagged as a follow-up consideration. Behavioral test (CPU oracle) pins the math contract: - shrink_and_perturb(α, σ) on X = α × X + (1−α) × N(0, σ) - with-fix vs without-fix differ by α × (W_best − W_curr) - cold-start (best == current) is bit-identical between orderings GPU-level kernel coverage stays in compile_training_kernels smoke and the L40S 30-epoch validation gating SP18 Phase 1. Pre-commit Invariant 7: docs/dqn-wire-up-audit.md updated with full rationale, state-interaction analysis, lifecycle notes, and the preserved cross-pearl invariants. Per: - feedback_no_partial_refactor (call-ordering + test + audit-doc atomic) - feedback_no_legacy_aliases (reuses existing API unchanged) - feedback_wire_everything_up (restore_best_gpu_params gains second cold-path consumer) - pearl_no_host_branches_in_captured_graph (runs outside graph capture)
This commit is contained in:
jgrusewski
@@ -9656,3 +9656,137 @@ rmsnorm cubin load: DriverError(CUDA_ERROR_NO_BINARY_FOR_GPU)
|
||||
**Proper fix**: extend cache key in `scripts/argo-train.sh` (or workflow
|
||||
template) to include `cuda-compute-cap`, e.g., `/data/bin/$SHORT_SHA-sm$CC/`.
|
||||
Filed as future infrastructure cleanup.
|
||||
|
||||
## SP18 v2 fold-transition fix: restore-best precedes shrink-and-perturb (2026-05-09)
|
||||
|
||||
### Bug
|
||||
|
||||
`train_walk_forward` in `crates/ml/src/trainers/dqn/trainer/mod.rs` called
|
||||
`self.reset_for_fold().await?` directly at the fold boundary, which
|
||||
applies `shrink_and_perturb(α=0.8, σ=0.01)` to whatever `params_flat`
|
||||
holds. At end-of-fold, `params_flat` carries the LATEST-EPOCH decayed
|
||||
weights (e.g. HD5, Sharpe ~60), NOT the best-Sharpe snapshot saved
|
||||
mid-fold (e.g. HD2, Sharpe ~91). So fold N+1 starts from
|
||||
`0.8 × decayed + noise(σ=0.01)` and carries the within-fold edge-decay
|
||||
forward into the next fold.
|
||||
|
||||
The vtj9r run audit surfaced this — Sharpe peaks degraded fold-over-fold
|
||||
in a pattern consistent with carrying forward post-peak weights. Calling
|
||||
`restore_best_gpu_params()` before `reset_for_fold` recovers the best
|
||||
snapshot saved by `handle_epoch_checkpoints_and_early_stopping` whenever
|
||||
`epoch_sharpe > self.best_sharpe`.
|
||||
|
||||
### The fix (call-ordering, additive)
|
||||
|
||||
Atomic insertion at the fold-transition site (before
|
||||
`reset_for_fold().await`):
|
||||
|
||||
```rust
|
||||
if let Err(e) = self.restore_best_gpu_params() {
|
||||
tracing::debug!(
|
||||
"Fold-transition restore-best skipped (no snapshot yet): {e}"
|
||||
);
|
||||
} else {
|
||||
tracing::info!(
|
||||
"Fold-transition: restored best-Sharpe GPU params before \
|
||||
shrink-and-perturb (fold {} → {})",
|
||||
fold_idx, fold_idx + 1,
|
||||
);
|
||||
}
|
||||
|
||||
// Then the existing reset_for_fold call runs:
|
||||
// 1. shrink_and_perturb(α=0.8, σ=0.01) on (now-best) params_flat
|
||||
// 2. sync_target_from_online() — target ← (now-best) online
|
||||
// 3. reset_adam_state() on every optimizer
|
||||
self.reset_for_fold().await?;
|
||||
```
|
||||
|
||||
`restore_best_gpu_params` is a thin wrapper over
|
||||
`fused_ctx.restore_best_params`, which:
|
||||
1. DtoD-copies `best_params_snapshot → params_buf`
|
||||
2. Unflattens into individual weight tensors
|
||||
3. Records a stream event (no host sync)
|
||||
|
||||
`reset_for_fold` runs on the same trainer stream, so the DtoD copy +
|
||||
unflatten complete before `shrink_and_perturb` reads `params_flat`.
|
||||
|
||||
### Cold-start guard
|
||||
|
||||
If `best_params_snapshot` is uninitialized (first fold or any fold
|
||||
without a Sharpe improvement yet), `restore_best_params` returns
|
||||
`Err("No best params snapshot saved")`. The fix swallows that error
|
||||
with a `debug!` log — fold transition then runs S&P on whatever
|
||||
`params_flat` currently holds, matching the pre-fix behavior. No
|
||||
regression on the cold-start path.
|
||||
|
||||
### State interaction with `reset_for_fold`
|
||||
|
||||
`restore_best_gpu_params` writes `params_flat` ONLY (online weights);
|
||||
it does NOT restore Adam state (m_buf/v_buf), target params, or any
|
||||
other optimizer state. `reset_for_fold` then:
|
||||
|
||||
- Calls `shrink_and_perturb` on the restored (best) `params_flat`
|
||||
- Calls `sync_target_from_online()` — hard-sync target ← restored online
|
||||
- Resets Adam state on main DQN, IQN head, TLOB, IQL, IQL-low,
|
||||
attention, VSN — fresh momentum from zero across all optimizers
|
||||
|
||||
These are non-conflicting: restore overwrites online weights; reset
|
||||
clears the OTHER state categories (Adam, target). Order is correct.
|
||||
|
||||
### `best_params_snapshot` lifecycle
|
||||
|
||||
The snapshot lives on `FusedTrainingCtx` (constructor-init `None`,
|
||||
allocated lazily on first `save_best_params` call) and is **NEVER
|
||||
cleared at fold boundary**. The trainer-side `self.best_sharpe` IS
|
||||
reset to `NEG_INFINITY` in `reset_for_fold` (so the first improvement in
|
||||
fold N+1 will overwrite the snapshot), but until then, the snapshot
|
||||
holds whichever fold most recently saved a peak.
|
||||
|
||||
This is the intended training-scoped behavior: peak weights from any
|
||||
fold can serve as the next fold's starting point. If strict
|
||||
within-fold semantics are wanted, the snapshot would need explicit
|
||||
clear at the fold boundary — flagged as a follow-up consideration.
|
||||
|
||||
### Behavioral test
|
||||
|
||||
`crates/ml/tests/sp18_fold_transition_test.rs` (NEW) — CPU oracle
|
||||
encoding the math contract:
|
||||
|
||||
- `shrink_and_perturb(α, σ) on X = α × X + (1−α) × N(0, σ)`
|
||||
- with-fix: `α × W_best + (1−α) × N`
|
||||
- without-fix: `α × W_curr + (1−α) × N`
|
||||
- diff: `α × (W_best − W_curr)`
|
||||
|
||||
The test asserts that with-fix ≠ without-fix when the best snapshot
|
||||
differs from current weights, and that the residual `out − α × W_best`
|
||||
falls within Wilson 95% CI of `(1−α) × σ × √N` for the noise term.
|
||||
Cold-start case (best == current) is bit-identical between the two
|
||||
orderings — pins the no-snapshot fallback as a safe no-op.
|
||||
|
||||
GPU-level kernel coverage stays in the existing
|
||||
`compile_training_kernels` smoke + L40S 30-epoch validation that
|
||||
gates the SP18 Phase 1 dispatch.
|
||||
|
||||
### Cross-pearl invariants preserved
|
||||
|
||||
- `feedback_no_partial_refactor`: call-ordering fix + behavioral test
|
||||
+ audit-doc update + pre-commit Invariant 7 satisfied in one atomic
|
||||
commit.
|
||||
- `feedback_no_legacy_aliases`: no new `_via_pinned` helpers; reuses
|
||||
existing `restore_best_gpu_params` API unchanged.
|
||||
- `feedback_wire_everything_up`: the previously single-consumer
|
||||
`restore_best_gpu_params` (used only by PLATEAU_EXHAUSTED) gains a
|
||||
second cold-path consumer at the fold transition.
|
||||
- `feedback_no_htod_htoh_only_mapped_pinned`: the underlying DtoD
|
||||
copy + unflatten is mapped-pinned-compatible; no new HtoD/HtoH paths
|
||||
introduced.
|
||||
- `pearl_no_host_branches_in_captured_graph`: the restore call runs
|
||||
outside CUDA Graph capture (epoch boundary, between folds).
|
||||
|
||||
### Files changed
|
||||
|
||||
| File | LOC delta | Purpose |
|
||||
|------|-----------|---------|
|
||||
| `crates/ml/src/trainers/dqn/trainer/mod.rs` | +49 / -0 | Insert `restore_best_gpu_params()` before `reset_for_fold()` at fold-transition site, with cold-start guard + commentary |
|
||||
| `crates/ml/tests/sp18_fold_transition_test.rs` | NEW | CPU oracle test for the math contract (3 cases) |
|
||||
| `docs/dqn-wire-up-audit.md` | +N / -0 | This entry |
|
||||
|
||||
Reference in New Issue
Block a user