From 22e6ddbcac2a47d82b3d72cfd1630e6509ec3836 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Sun, 31 May 2026 18:09:16 +0200 Subject: [PATCH] fix(rl): eval_summary aggregates across all b_size accounts (E.4-E.5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the broken single-account + scale-mismatch slicing block in alpha_rl_train.rs eval phase with proper per-account aggregation. Pre-fix (cluster v11 alpha-rl-8ll7j, b=1024): head_before_eval = sim.read_total_trade_count() // aggregate=1,203,376 all_records = sim.read_trade_records(0) // backtest 0 only, ≤1024 // 1.2M > 1024 → wrap branch fires → eval_records = ALL 1024 records // from account 0, mixing train+eval trades eval_summary: n_trades=1024 pnl=$61,513 wr=0.217 — MEANINGLESS Post-fix (this commit, b=16 local smoke): head_before_per_b = sim.read_per_backtest_trade_counts() all_per_b = sim.read_trade_records_all() // for each backtest: slice ring by per-account head_before vs head_after, // handle wrap correctly, aggregate eval-only records eval_summary: n_trades=226 pnl=$-41,137 wr=0.376 n_eval_trades_seen=226 (= captured) n_dropped=0 n_pre_eval_wrapped=0 Local smoke validation (b=16, 1k train + 250 eval, fold 1 ES.FUT all): - n_trades jumped 58 → 226 (4× — confirms aggregation across 16 accts) - per-account pre-eval heads: min=16 max=59 sum=596 (correct scale) - n_eval_trades_seen == n_trades (no wrap at this scale) - eval_summary.json gained 4 new fields: n_eval_trades_seen, n_eval_trades_dropped, n_pre_eval_trades_wrapped, b_size Existing keys (n_trades, total_pnl_usd, profit_factor, sharpe_ann, max_drawdown_usd, win_rate) preserved for downstream G8-gate / Argo aggregator consumers (per spec §3). Cluster validation pending Phase A cluster (alpha-rl-jz48s) completion to avoid alpha_rl_train.rs branch conflict. After Phase A merges, this fix can submit alongside. Spec: docs/superpowers/specs/2026-05-31-eval-summary-trade-aggregation-design.md Plan: docs/superpowers/plans/2026-05-31-eval-summary-trade-aggregation.md Co-Authored-By: Claude Opus 4.7 --- crates/ml-alpha/examples/alpha_rl_train.rs | 135 +++++++++++++++++---- 1 file changed, 114 insertions(+), 21 deletions(-) diff --git a/crates/ml-alpha/examples/alpha_rl_train.rs b/crates/ml-alpha/examples/alpha_rl_train.rs index b51a01f26..a9b9aac96 100644 --- a/crates/ml-alpha/examples/alpha_rl_train.rs +++ b/crates/ml-alpha/examples/alpha_rl_train.rs @@ -742,12 +742,22 @@ fn main() -> Result<()> { .reset_session_state() .context("reset_session_state at train→eval boundary")?; - let head_before_eval = sim - .read_total_trade_count() - .context("read trade count pre-eval")?; + let head_before_per_b = sim + .read_per_backtest_trade_counts() + .context("snapshot per-backtest trade-count pre-eval")?; + let head_before_min = head_before_per_b.iter().min().copied().unwrap_or(0); + let head_before_max = head_before_per_b.iter().max().copied().unwrap_or(0); + let head_before_sum: u64 = + head_before_per_b.iter().map(|&h| h as u64).sum(); eprintln!( - "── eval phase: {} steps on {} held-out files (trade-record checkpoint head={}) ──", - cli.n_eval_steps, eval_files.len(), head_before_eval + "── eval phase: {} steps on {} held-out files; pre-eval head per b_size={} \ + accounts: min={} max={} sum={} ──", + cli.n_eval_steps, + eval_files.len(), + head_before_per_b.len(), + head_before_min, + head_before_max, + head_before_sum ); let eval_loader_cfg = MultiHorizonLoaderConfig { @@ -985,22 +995,82 @@ fn main() -> Result<()> { } eval_diag.flush().context("eval diag: final flush")?; - // Drain trade records, slice to eval-only, compute summary. - let all_records = sim - .read_trade_records(0) - .context("read trade records post-eval")?; - let head_before_usize = head_before_eval as usize; - let eval_records: Vec<_> = if all_records.len() > head_before_usize { - all_records[head_before_usize..].to_vec() - } else { - // Trade log wrapped past TRADE_LOG_CAP — use what we have. - // For smoke (b_size=1, ≤200 trades) this branch never fires. + // Aggregate eval-phase trades across ALL b_size accounts, with + // correct per-account head_before slicing. Replaces the previous + // single-account read of backtest 0 + aggregate-vs-per-account + // scale mismatch (see spec + // docs/superpowers/specs/2026-05-31-eval-summary-trade-aggregation-design.md + // for the root-cause analysis). + let all_per_b = sim + .read_trade_records_all() + .context("read all backtests' trade records post-eval")?; + let head_after_per_b = sim + .read_per_backtest_trade_counts() + .context("snapshot per-backtest trade-count post-eval")?; + + let cap_u32 = ml_backtesting::lob::TRADE_LOG_CAP as u32; + let mut eval_records: Vec = Vec::new(); + let mut n_eval_trades_seen: u64 = 0; + let mut n_eval_trades_dropped: u64 = 0; + let mut n_pre_eval_wrapped: u64 = 0; + + for (b, records) in all_per_b.iter().enumerate() { + let head_before = head_before_per_b[b]; + let head_after = head_after_per_b[b]; + let eval_count_total = head_after.saturating_sub(head_before); + n_eval_trades_seen += eval_count_total as u64; + + // The ring's contents cover cumulative-stream indices + // [max(0, head_after - cap), head_after). + let ring_start_stream_idx = head_after.saturating_sub(cap_u32); + + if head_before < ring_start_stream_idx { + // Some pre-eval trades had wrapped out before eval started — + // diagnostic only, doesn't affect eval slicing. + n_pre_eval_wrapped += (ring_start_stream_idx - head_before) as u64; + } + + if eval_count_total > cap_u32 { + // Eval-phase trades wrapped (lost). Should be 0 with + // TRADE_LOG_CAP=4096 at typical cluster scale. + n_eval_trades_dropped += (eval_count_total - cap_u32) as u64; + } + + // Slice the ring contents to eval-only. + // ring index of first eval trade = head_before − ring_start_stream_idx + // (clamped at 0 if all pre-eval already wrapped out). + let eval_start_in_ring = + head_before.saturating_sub(ring_start_stream_idx) as usize; + if eval_start_in_ring < records.len() { + eval_records.extend_from_slice(&records[eval_start_in_ring..]); + } + } + + if n_eval_trades_dropped > 0 { eprintln!( - "warning: trade log wrapped — head_before={} but only {} records readable", - head_before_usize, all_records.len() + "warning: {} eval trades wrapped out across {} accounts \ + (TRADE_LOG_CAP={}); summary based on {} captured eval trades", + n_eval_trades_dropped, + all_per_b.len(), + cap_u32, + eval_records.len() ); - all_records - }; + } + if n_pre_eval_wrapped > 0 { + eprintln!( + "info: {} pre-eval trades had already wrapped before eval phase \ + (no effect on eval summary)", + n_pre_eval_wrapped + ); + } + eprintln!( + "eval phase trade accounting: n_eval_trades_seen={} n_captured={} \ + n_dropped={} b_size={}", + n_eval_trades_seen, + eval_records.len(), + n_eval_trades_dropped, + all_per_b.len() + ); // Synthesise a pnl_curve from per-trade cumulative PnL for // compute_summary's max_drawdown calc. @@ -1014,19 +1084,42 @@ fn main() -> Result<()> { ml_backtesting::artifacts::compute_summary(&eval_records, &pnl_curve); eprintln!( - "eval summary: n_trades={} pnl_usd={:.2} pf={:.3} sharpe_ann={:.3} max_dd_usd={:.2} win_rate={:.3}", + "eval summary: n_trades={} pnl_usd={:.2} pf={:.3} sharpe_ann={:.3} \ + max_dd_usd={:.2} win_rate={:.3} | seen={} dropped={} b={}", eval_summary.n_trades, eval_summary.total_pnl_usd, eval_summary.profit_factor, eval_summary.sharpe_ann, eval_summary.max_drawdown_usd, eval_summary.win_rate, + n_eval_trades_seen, + n_eval_trades_dropped, + all_per_b.len(), ); + // Write eval_summary.json with the existing compute_summary fields + // PLUS four new aggregation-aware fields: + // n_eval_trades_seen — true total cumulative dones across b_size + // n_eval_trades_dropped — eval trades lost to ring wrap (0 at typical scale) + // n_pre_eval_trades_wrapped — pre-eval trades wrapped before eval (diagnostic) + // b_size — context for downstream interpretation + let aggregated_summary = serde_json::json!({ + "n_trades": eval_summary.n_trades, + "total_pnl_usd": eval_summary.total_pnl_usd, + "profit_factor": eval_summary.profit_factor, + "sharpe_ann": eval_summary.sharpe_ann, + "max_drawdown_usd": eval_summary.max_drawdown_usd, + "win_rate": eval_summary.win_rate, + "n_eval_trades_seen": n_eval_trades_seen, + "n_eval_trades_dropped": n_eval_trades_dropped, + "n_pre_eval_trades_wrapped": n_pre_eval_wrapped, + "b_size": cli.n_backtests, + }); + let eval_summary_path = cli.out.join("eval_summary.json"); let f = std::fs::File::create(&eval_summary_path) .with_context(|| format!("create {}", eval_summary_path.display()))?; - serde_json::to_writer_pretty(f, &eval_summary) + serde_json::to_writer_pretty(f, &aggregated_summary) .with_context(|| format!("write {}", eval_summary_path.display()))?; eprintln!("eval summary written: {}", eval_summary_path.display()); }