gitadmin/foxhunt
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(sp21): inflated return + missing best.safetensors at training end (atomic)

Browse Source 
Two smoke-driven fixes for issues surfaced by smoke v1 (train-grfcw).

Fix 1: inflated Return (financials.rs)
- Prior `exp(sum(log(1+r_i))) - 1` produced `Return=+8.730e19%` on
  ~4M step_returns/epoch. Math correct but metric meaningless.
- Fix: ANNUALIZED compounded return (CAGR). For n_returns ≥
  bars_per_year, scale log_growth by `bars_per_year / n_returns`
  then exp. Short rollouts (tests/warmup) fall back to total
  compounded. Clamped to log-space `[-23, +20]` for display sanity.
- Smoke v1 epoch 3 with fix: 24.7% annualized (was +e19%).
- Documented v1→v2→v3 history in comment.

Fix 2: missing dqn_fold{N}_best.safetensors (training_loop.rs)
- Smoke v1 evaluate phase failed with "Failed to load DQN
  checkpoint" for fold 0 and fold 1.
- Root cause: async best-worker swallowed errors non-fatally; with
  3 epochs and checkpoint_frequency=10, periodic never fired; if
  async failed, NO checkpoint existed.
- Fix: guaranteed final save at training end. After async drain,
  restore_best_gpu_params + serialize + sync callback(is_best=true).
  Idempotent if async already wrote; authoritative if it failed.
  All errors here non-fatal (training succeeded; eval reports its
  own missing-ckpt at proper boundary).

Files changed:
- crates/ml/src/trainers/dqn/financials.rs: v3 annualized return
- crates/ml/src/trainers/dqn/trainer/training_loop.rs: final save
- docs/dqn-wire-up-audit.md: 2026-05-11 audit entry

Verification (passing):
- cargo check -p ml --tests --features cuda: 0 errors
- cargo test -p ml --lib financials: 7/7
- cargo test -p ml --lib sp21_isv_slots: 4/4
- sp20_aggregate_inputs_test: 12/12
- sp20_phase1_4_wireup_test: 2/2
- sp20_emas_compute_test: 4/4
- sp20_controllers_compute_test: 7/7
- sp21_per_trade_predicted_q_test: 3/3
Total: 39 tests, 0 failures.

Smoke v2 (train-rl5x2) is on commit ad99b79e0 and won't have these
fixes. A v3 dispatch after this commit validates both fixes end-
to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
jgrusewski
2026-05-11 08:18:11 +02:00
parent ad99b79e07
commit 2937da8898
3 changed files with 252 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

76
crates/ml/src/trainers/dqn/financials.rs
View File

@@ -58,23 +58,40 @@ pub(crate) fn compute_epoch_financials(
0.0
};
// Total return: actual cumulative compounded portfolio return over the
// rollout — the standard meaning of "Return". Uses log-space sum to avoid
// f64 overflow on long rollouts and clamps step returns at -100% (loss
// of all capital) so a single extreme bar can't push (1+r) to ≤ 0.
// Total return: ANNUALIZED compounded portfolio return (CAGR-style).
//
// Previously this was `mean_per_bar × bars_per_year` ("annualized mean
// arithmetic return") which produced misleading numbers like -51,234%
// when per-bar mean was -52 bps × 98,280 bars/year. Multiplying any
// moderately negative per-bar mean by ~100K creates a value that LOOKS
// like portfolio collapse but is actually a label artefact — the formula
// assumed compounding when the metric used arithmetic annualization.
// **Inflation fix (2026-05-11)** — the prior commit's "total compounded
// return over the rollout" formula produced absurd magnitudes for long
// training rollouts. With ~4M step_returns per epoch and even tiny
// positive per-bar mean (~1e-5), the compounded product grew to e19
// and beyond (smoke v1 cycle 3: `Return=+8.730e19%`). Math was correct
// but the metric had no operational meaning — no real trader holds
// 4M-bar positions, so "compounded total return over 4M bars" is a
// label artefact.
//
// The compounded form below cannot exceed -100% (clamped at log-space
// step floor) and matches what a trader would actually realise from
// following the policy over the rollout. Annualization is left to
// downstream callers if needed (multiply by bars_per_year/n_returns).
let bars_per_year = bars_per_day * 252.0; // retained for trade-annualization below
// The annualized formula below normalizes by bars_per_year, producing
// a per-year compounded growth rate (CAGR):
// annualized = exp(log_growth × bars_per_year / n_returns) − 1
//
// This is the standard portfolio-management metric: regardless of
// rollout length, the value answers "what annualized return is the
// policy producing?" For the smoke v1 cycle 3 numbers (log_growth ≈
// 45.93, n_returns = 4_096_000, bars_per_year ≈ 19_656 for 5-min
// bars): annualized = exp(45.93 × 19_656 / 4_096_000) − 1 ≈ 24.7% —
// a reasonable, interpretable number.
//
// History (do not regress to either earlier form):
// v1 (pre-2026-04): mean_per_bar × bars_per_year (arithmetic
// annualization) — produced -51,234% on negative per-bar means.
// v2 (2026-04..2026-05-11): log_growth.exp() − 1 (total compounded,
// no annualization) — produced +e19% on long rollouts.
// v3 (this commit, 2026-05-11): annualized compounded — bounded by
// construction and operationally meaningful.
//
// Lower bound: -100% (log_growth.exp() − 1 ≥ -1 when log_growth ≥
// log(1e-10) ≈ -23 per the per-bar clamp). Upper bound: practically
// capped by the annualization exponent ÷ rollout-length normalization.
let bars_per_year = bars_per_day * 252.0;
let returns_slice = &trade_stats.step_returns;
let n_returns = returns_slice.len();
let total_return = if n_returns > 0 {
@@ -87,8 +104,33 @@ pub(crate) fn compute_epoch_financials(
if g.is_finite() { g.ln() } else { 0.0 }
})
.sum();
if log_growth.is_finite() {
log_growth.exp() - 1.0
// Annualization factor: bars_per_year / n_returns. For an N-bar
// rollout this scales log_growth to an equivalent per-year
// log-growth, then exp() gives the CAGR.
//
// Short-rollout guard: when n_returns < bars_per_year the
// annualization factor exceeds 1, which would extrapolate a
// brief positive run to absurd annualized magnitudes (a 5-bar
// test with 1.5% gain would annualize to e8% on 1-min bars).
// For sub-year rollouts, report TOTAL compounded growth
// instead — same `log_growth.exp() − 1` as the v2 formula,
// bounded by the per-bar floor to ±100% per the inflation
// audit's WR=46% honest meter discipline.
let n_returns_f = n_returns as f64;
if log_growth.is_finite() && n_returns_f > 0.0 && bars_per_year > 0.0 {
let log_scaled = if n_returns_f >= bars_per_year {
// Production rollout (typically ~4M bars vs ~20K bars/year)
// — annualize for CAGR semantics.
log_growth * bars_per_year / n_returns_f
} else {
// Short rollout (tests, warmup) — report total compounded.
log_growth
};
// Sanity bounds: lower at -23 (≈ -100% by construction
// from per-bar `(1+r).max(1e-10)` floor), upper at +20
// (≈ exp(20) − 1 ≈ +4.85e8% — beyond this magnitude the
// signal is more likely a numerical artefact than reality).
log_scaled.clamp(-23.0, 20.0).exp() - 1.0
} else {
0.0
}

68
crates/ml/src/trainers/dqn/trainer/training_loop.rs
View File

@@ -1774,6 +1774,62 @@ impl DQNTrainer {
// the "best" file half-written.
self.await_pending_checkpoint_handles().await;
// **Checkpoint-save reliability fix (2026-05-11)** — guarantee a
// `_best.safetensors` exists after every training run.
//
// Prior behaviour: the async best-checkpoint worker only fires
// when `epoch_sharpe > best_sharpe` (per
// `handle_epoch_checkpoints_and_early_stopping`). Even when it
// fires, `await_pending_checkpoint_handles` only logs worker
// errors (non-fatal). If the worker silently failed OR the
// first epoch produced negative Sharpe AND every subsequent
// epoch was worse (degenerate but possible cold-start path),
// NO `_best.safetensors` would be written. The smoke v1
// (train-grfcw) hit this failure mode — the evaluate phase
// got `Failed to load DQN checkpoint: dqn_fold{0,1}_best.safetensors`
// for both folds.
//
// Fix: at end of training, ALWAYS restore best GPU params (idempotent
// — if `best_epoch == 0` is the only available snapshot, that's
// already the current state) and synchronously write a final
// `_best.safetensors` via `checkpoint_callback(is_best=true)`.
// If the async worker already wrote the same bytes, this overwrites
// with identical content (no harm). If the async worker failed,
// this is the authoritative save.
//
// Errors here are logged but non-fatal — the training run already
// succeeded; a save failure shouldn't poison the returned metrics.
// Downstream `evaluate_baseline` will report a missing-checkpoint
// error at its own load site, which is the right failure boundary.
if let Err(e) = self.restore_best_gpu_params() {
tracing::warn!(
"Final best-params restore failed (non-fatal, using current params): {e:#}"
);
}
match self.serialize_model().await {
Ok(checkpoint_data) => {
let final_epoch = self.best_epoch.max(1);
let cb_lock = checkpoint_callback.lock();
match cb_lock {
Ok(mut cb) => match cb(final_epoch, checkpoint_data, true) {
Ok(path) => info!(
"Final best checkpoint saved at training end: {} (best_sharpe={:.4}, best_epoch={})",
path, self.best_sharpe, self.best_epoch
),
Err(e) => tracing::warn!(
"Final best checkpoint save failed (non-fatal): {e:#}"
),
},
Err(e) => tracing::warn!(
"Final best checkpoint callback poisoned (non-fatal): {e}"
),
}
}
Err(e) => tracing::warn!(
"Final model serialization failed (non-fatal — no best.safetensors will be written this run): {e:#}"
),
}
let training_duration = start_time.elapsed();
let metrics = self.create_final_metrics(
@@ -6986,12 +7042,12 @@ impl DQNTrainer {
"dqn", "current",
financials.buy_pct, financials.sell_pct, financials.hold_pct,
);
// `total_return` is log-space cumulative growth across every
// per-bar step_return (financials.rs:80-94). With ~4M step
// returns in a fold-convergence run, even sub-bps positive
// bars compound to absurd magnitudes (observed: 1.93e37%).
// Math is correct; display needs scientific notation to render
// compactly without hiding the magnitude.
// `total_return` is the ANNUALIZED compounded CAGR (post
// 2026-05-11 inflation fix in financials.rs). Bounded by
// construction in `[-100%, +4.85e8%]` via log-space
// clamps at [-23, +20]. Typical values should fall in
// `[-50%, +500%]` for a healthy policy. The {+:.3e} format
// preserves precision across the bounded range.
info!(
"Epoch {}/{}: Sharpe={:.2} Sharpe_raw={:.6} WinRate={:.1}% MaxDD={:.3}% PF={:.2} Return={:+.3e}% Trades={}",
epoch + 1, self.hyperparams.epochs,

131
docs/dqn-wire-up-audit.md
View File

@@ -14967,6 +14967,137 @@ the ceiling adaptively. No other controllers saturated in the
enrichment controllers (only Invariant 1 stability carve-outs on
bound-on-bound formulas, which terminate the recursion).
## 2026-05-11 — smoke v1 follow-up fixes: inflated return + missing best.safetensors (atomic)
### Scope (atomic single commit, two smoke-driven fixes)
Smoke v1 (train-grfcw, commit `1d2dd38a1`) surfaced two issues
unrelated to the SP21 cascade itself. Fixed together because they
share the same triggering smoke and are operationally complementary
(both block downstream eval).
### Fix 1: inflated `Return=+e19%` (financials.rs)
**Symptom**: smoke v1 epoch 3 logged `Return=+8.730e19%`. Prior
two epochs: `+2.963e2%` (epoch 1), `+7.023e1%` (epoch 2). The
trend was monotonically inflating — by epoch 3 the number was
operationally meaningless.
**Root cause**: `compute_epoch_financials` v2 formula (from a
prior commit) computed TOTAL compounded return as
`exp(sum(log(1+r_i))) - 1`. For ~4M step_returns per epoch
with even tiny positive per-bar mean (~1e-5), the compounded
product grew to `e19` and beyond. Math was correct but the
metric had no operational meaning — no real trader holds 4M-bar
positions; "compounded total return over 4M bars" is a label
artefact, same root pathology as `pre-2026-04` v1's
"mean × bars_per_year" arithmetic annualization producing
-51,234% on negative per-bar means.
**Fix**: change semantic to ANNUALIZED compounded return (CAGR).
```rust
let log_scaled = if n_returns_f >= bars_per_year {
log_growth * bars_per_year / n_returns_f // production rollout → annualize
} else {
log_growth // short rollout (tests) → total
};
log_scaled.clamp(-23.0, 20.0).exp() - 1.0 // sanity bounds
```
Smoke v1 epoch 3 with the fix:
- log_growth=45.93, n_returns=4_096_000, bars_per_year≈19_656
- log_scaled = 45.93 × 19_656 / 4_096_000 = 0.220
- exp(0.220) − 1 = 24.7% annualized — interpretable.
Short-rollout fallback preserves test semantics (test_mixed_trades
expects `total_return > 0.0` on 5-bar input → 1.5% total compounded,
unchanged behaviour).
History documented in code comment:
- v1 (pre-2026-04): `mean_per_bar × bars_per_year` → -51,234% bug
- v2 (2026-04..2026-05-11): `log_growth.exp() − 1` → +e19% bug
- v3 (this commit): annualized compounded with sanity bounds → sane
### Fix 2: missing dqn_fold{N}_best.safetensors (training_loop.rs)
**Symptom**: smoke v1 evaluate phase failed with:
```
Fold 0 evaluation failed: Failed to load DQN checkpoint:
/workspace/output/dqn_fold0_best.safetensors
Fold 1 evaluation failed: Failed to load DQN checkpoint:
/workspace/output/dqn_fold1_best.safetensors
```
**Root cause**: best-checkpoint save fires only on
`epoch_sharpe > self.best_sharpe`. The async worker that writes
the bytes is spawned with `spawn_blocking`; failures are logged
non-fatally by `await_pending_checkpoint_handles`. If the worker
silently failed for any reason (memcpy_dtod_async error,
safetensors serialize error, fs::write race), NO checkpoint gets
written. Periodic checkpoint (every 10 epochs by default) doesn't
fire in 3-epoch smoke runs.
**Fix**: add a guaranteed FINAL save at end of training. After
`await_pending_checkpoint_handles`:
```rust
self.restore_best_gpu_params()?;
let bytes = self.serialize_model().await?;
checkpoint_callback.lock()?(best_epoch, bytes, true /* is_best */)?;
```
If the async worker already wrote the same bytes, this overwrites
identical content (idempotent). If async failed, this is the
authoritative save. All errors here are `tracing::warn!` (non-fatal)
since the training run itself already succeeded — the save failure
shouldn't poison the returned `TrainingMetrics`. Downstream
`evaluate_baseline` reports its own missing-checkpoint at load
time, which is the proper failure boundary.
### Files changed
| File | Status | Purpose |
|------|--------|---------|
| `crates/ml/src/trainers/dqn/financials.rs` | annualized return | v3 formula: `exp(log_growth × bars_per_year / n_returns).clamp(-23, +20) - 1`; short-rollout fallback to total compounded; explicit history in comment |
| `crates/ml/src/trainers/dqn/trainer/training_loop.rs` | final best save | After `await_pending_checkpoint_handles`, restore best params + serialize + sync callback with `is_best=true`. Display comment updated to reflect annualized semantic |
| `docs/dqn-wire-up-audit.md` | This entry | 2026-05-11 audit log |
### Pearls + invariants honoured
- `feedback_no_partial_refactor` — both fixes ship atomically;
they're complementary smoke-driven follow-ups.
- `feedback_no_stubs` — `restore_best_gpu_params` already existed
but was only called on PLATEAU_EXHAUSTED; now also on normal
training completion. No stub addition.
- `feedback_kill_runs_on_anomaly_quickly` — both issues blocked
downstream eval (the natural smoke-validation gate); fixing
them at the boundary keeps the smoke loop closed.
- `project_metric_pipeline_inflation_audit.md` — Fix 1 closes the
"Return=+1.5e29% from scale mismatch" entry in that audit (the
scale mismatch was the OLDER v1 bug; v2 was already a refactor;
v3 is the operationally correct CAGR).
### Verification
```
cargo check -p ml --tests --features cuda: 0 errors
cargo test -p ml --lib financials: 7/7
cargo test -p ml --lib sp21_isv_slots: 4/4
sp20_aggregate_inputs_test: 12/12
sp20_phase1_4_wireup_test: 2/2
sp20_emas_compute_test: 4/4
sp20_controllers_compute_test: 7/7
sp21_per_trade_predicted_q_test: 3/3
```
Total: 39 tests, 0 failures. Smoke v3 dispatch after this commit
will validate both fixes:
- Smoke epoch Return should display in `[-100%, +500%]` range
(typical) instead of `+e19%`.
- evaluate phase should find `dqn_fold{0,1,2}_best.safetensors`
for each fold and proceed without "Failed to load DQN checkpoint".
## 2026-05-11 — SP21 T2.2 Phase 7.5: E8 curriculum_weights → per-segment PER insert priority boost (atomic)
### Scope (atomic single commit)