From 0efdee4b0c9f3a2b8b7cef666786c92796df2547 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Sat, 23 May 2026 09:34:07 +0200 Subject: [PATCH] =?UTF-8?q?docs(plans):=20rebuild=20plan=20v2=20=E2=80=94?= =?UTF-8?q?=20strict=20memory=20+=20GPU-oracle=20gates?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audited the v1 plan against the full project memory catalog. Found several rule violations and gaps: A3 (was: 'host memcpy_htod canonical defaults to ISV[400..406]') → violated feedback_no_htod_htoh_only_mapped_pinned ('tests not exempt') and short-circuited pearl_first_observation_bootstrap. Replaced with launch-once-at-init: each controller fires once with sentinel-zero input, kernel's first-observation-bootstrap path writes the canonical value. No host write to ISV. Canonical pattern across the codebase. G1-G7 gates (was: 'matches host reference' for argmax_expected_q) → violated feedback_no_cpu_test_fallbacks. Replaced every CPU oracle with GPU oracle: analytical synthetic inputs, property assertions, cross-kernel validation only. Added explicit catalog of memory rules the rebuild MUST honor (30+ rules grouped by domain). Every R-phase + every architectural decision now cites which rules it applies. Added A9 (PER actually wired into step) per feedback_always_per — the flawed branch had ReplayBuffer struct but never sampled from it. Added new ISV slots for 7 EMA inputs (RL_*_EMA_INDEX) → RL_SLOTS_END extends to 424. The controllers' inputs live on device, not host. Added cluster smoke discipline section: per pearl_single_window_oos the G8 backtest gate requires >=3 walk-forward folds. Per feedback_kill_runs_on_anomaly_quickly the dispatcher kills on NaN / ISV saturation / kernel hang. Per pearl_q_spread misaligned, the dispatcher MUST NOT kill on Q_SPREAD. Per feedback_argo_template_must_apply the dispatcher refuses to submit if template not applied since edit. Per feedback_push_before_deploy the dispatcher hard-errors if local HEAD != origin HEAD. Added pre-cluster validation checklist (R9 prerequisite): full local-CUDA gate matrix that must be green before push. Reconciled feedback_mbp10_mandatory: --mbp10-data-dir is mandatory; --trades-data-dir is flagged as a gap (ml-alpha's MultiHorizonLoader does not currently consume a separate trades stream — OFI is derived from MBP-10 snapshot deltas). Either wire trades in a future plan or ship explicitly without; the rebuild does the latter and flags it. Plan grew from 381 to ~580 lines because the memory audit exposed several decisions that needed explicit treatment instead of implicit 'follow project pattern' references. Co-Authored-By: Claude Opus 4.7 --- ...026-05-23-integrated-rl-trainer-rebuild.md | 830 ++++++++++++------ 1 file changed, 550 insertions(+), 280 deletions(-) diff --git a/docs/superpowers/plans/2026-05-23-integrated-rl-trainer-rebuild.md b/docs/superpowers/plans/2026-05-23-integrated-rl-trainer-rebuild.md index 894744eea..5e44b6cbf 100644 --- a/docs/superpowers/plans/2026-05-23-integrated-rl-trainer-rebuild.md +++ b/docs/superpowers/plans/2026-05-23-integrated-rl-trainer-rebuild.md @@ -4,122 +4,148 @@ > on branch `ml-alpha-phase-f-g-flawed` (commits 99a125cdb..b3808a5ac). > Baseline: `ml-alpha-phase-a` reset to commit `9114374d2` > (Phase E.3b activation). +> +> **Discipline:** Smokes are expensive ($5–20 / 30–120 min L40S each) +> and producing G8 numbers from a buggy trainer wastes both cluster +> credit and gate-evaluation cycles. Every rule below is non-negotiable. +> Every gate is falsifiable locally before any `argo submit`. ## Why this rebuild -The prior Phase F + G attempt landed 9 commits that shipped a trainer -with multiple production-blocking defects. The defects were not bugs in -the new code so much as **propagation of pre-existing violations** in -`step_with_lobsim` plus **incomplete wiring of the ISV adaptive-control -infrastructure**. The convergence-gate fixtures (`dqn_reward_signal`, -`ppo_reward_signal`, then named `dqn_toy` / `ppo_toy`) hid every defect -because their `MockLobEnv` has `done = true` every step, reward in -`[-1, +1]`, and horizon = 1. The cluster-bound smoke was the only -mechanism that would have exposed the problems — at $5–20 / 30–120 min -each. +The prior Phase F + G attempt landed 9 commits with multiple +production-blocking defects. None of the defects were exotic bugs; +all of them were direct violations of pre-existing project memory +that I missed because the `MockLobEnv` "toy bandit" fixture has +`done = true` every step, reward ∈ `[-1, +1]`, and horizon = 1 — +which hides every failure mode the production cluster will see. -### Defects in the flawed branch +### Defect table — each row maps to a memory rule the flawed branch violated -1. **ISV[400..406] uninitialised in production paths.** The trainer - `alloc_zeros` the ISV array; only `rl_lr_controller` was launched. - Production consumers read sentinel `0`: - - `bellman_target_projection.cu:133` reads γ = 0 → Bellman target - reduces to `r + 0·V(s_{t+1}) = r`. Q-head learns immediate-reward - only, zero credit assignment. - - `ppo_clipped_surrogate.cu:205,305` reads ε = 0 → `clip(r, 1, 1) = 1` - always. PPO trust region collapses. - - `ppo_clipped_surrogate.cu:210` reads entropy coef = 0 → no entropy - bonus, policy collapses to deterministic immediately. +| # | Defect | Memory rule violated | +|---|--------|----------------------| +| 1 | ISV[400..406] uninitialised in production paths (γ=0, ε=0, entropy=0 in kernels) | `feedback_wire_everything_up`, `feedback_isv_for_adaptive_bounds` | +| 2 | `rl_reward_scale_controller` drifts to 1e3 on no-trade steps | `pearl_first_observation_bootstrap` | +| 3 | 6 controllers exist as .cu files but never launched | `feedback_wire_everything_up`, `feedback_no_partial_refactor` | +| 4 | Target net never soft-updated (τ has no consumer) | `feedback_no_stubs`, `feedback_wire_everything_up` | +| 5 | `step_with_lobsim` has 5 DtoH + host argmax + host EMA + host advantage loops | `feedback_cpu_is_read_only`, `pearl_cold_path_no_exception_to_gpu_drives` | +| 6 | PER buffer exists in `src/rl/replay.rs` but trainer never samples from it | `feedback_always_per` | +| 7 | "Toy bandit" framing leaked into production (`alpha_rl_train.rs` shipped with `next_snapshots = snapshots` until caught mid-review) | `feedback_no_stubs`, `feedback_no_quickfixes`, `feedback_extending_existing_code_audits_for_existing_violations` | +| 8 | No NaN / divergence abort in production CLI | `feedback_stop_on_anomaly`, `feedback_kill_runs_on_anomaly_quickly` | +| 9 | Convergence-gate fixtures (`dqn_toy.rs`, `ppo_toy.rs`) used `MockLobEnv` with state-invariant rewards — intrinsically can't catch defects #1, #2, #5 | `pearl_tests_must_prove_not_lock_observations`, `feedback_no_cpu_test_fallbacks` | -2. **`rl_reward_scale_controller` drifts to `1e3` on no-trade steps.** - The launcher fired every step regardless of `mean_abs_pnl_ema` - readiness; with `ema = 0` and `prev = REWARD_SCALE_BOOTSTRAP = 1.0`, - the kernel computed `target = 1/max(0, 1e-3) = 1000`, Wiener-blended - toward it, and ramped the scale to `1e3` over the first ~10 hold - steps. Toy fixture hid this because `done = true` every step. +### Root-cause pattern -3. **6 controllers exist as .cu files but never launched.** - `rl_gamma`, `rl_target_tau`, `rl_ppo_clip`, `rl_entropy_coef`, - `rl_rollout_steps`, `rl_per_alpha`. The build system compiles their - cubins; no Rust launcher fires them. ISV bootstrap (defect #1) - would have masked this even if the controllers had been wired. +Defects #1, #3, #5, #6 are all the same shape: **the pre-existing +Phase E.3b code already violated current memory rules, and the +flawed Phase F extended the pattern instead of flagging it.** The new +memory pearl `feedback_extending_existing_code_audits_for_existing_violations` +exists specifically to prevent this in future work. -4. **Target-net soft update missing.** `DqnHead` owns `target_w_d / - target_b_d` but no kernel updates them. The "target network" is - actually the initial random init for the entire run — no soft Polyak - update, no hard sync. Double-DQN Bellman target uses an - untrained-static target. +## Strict memory rules this rebuild MUST honor -5. **`feedback_cpu_is_read_only` violation in `step_with_lobsim`.** - Host code: - - 5 DtoH copies per step (Q / V / π logits at h_t, V / Q at h_{t+1}) - - Host-side softmax + Thompson categorical sample + argmax loops - over `[B][N_ACTIONS][Q_N_ATOMS]` - - Host EMA updates for `mean_abs_pnl_ema` - - Host advantage / return computation - - HtoD copies for `actions`, `rewards`, `dones` - The pre-existing Phase E.3b code already had this pattern; the - flawed Phase F extended it instead of rebuilding it as GPU-resident - per `pearl_cold_path_no_exception_to_gpu_drives`. +Explicit catalog, every rule named below is gating. If the rebuild +diverges from any of these the work does NOT ship. -6. **"Toy" framing in production code.** Test files `dqn_toy.rs` / - `ppo_toy.rs`, fixture constructor `MockLobEnv::toy_bandit()`, and - pervasive "toy bandit" / "for the toy bandit smoke" comments leaked - into production source. Worse: `alpha_rl_train.rs` shipped with - `next_snapshots = snapshots` — production code applying a - toy-fixture simplification — until that one issue got caught - mid-review. +### GPU/CPU contract +- `feedback_cpu_is_read_only` — no CPU compute, no CPU forwards, no GPU↔CPU roundtrips on hot path +- `feedback_no_htod_htoh_only_mapped_pinned` — mapped-pinned only; tests not exempt +- `feedback_no_atomicadd` — block tree-reduce / per-batch scratch + `reduce_axis0`, never atomicAdd +- `feedback_no_cpu_test_fallbacks` — GPU oracle tests only; no CPU reference implementations +- `feedback_no_nvrtc` — pre-compiled cubins via `build.rs` +- `pearl_no_host_branches_in_captured_graph` — host branches inside captured graphs break recording +- `pearl_cold_path_no_exception_to_gpu_drives` — "cold path" is not a license for CPU compute +- `pearl_fused_per_group_statistics_oracle` — K groups × N stats = ONE fused kernel +- `pearl_build_rs_rerun_if_env_changed` — every `std::env::var()` paired with `cargo:rerun-if-env-changed` -7. **No NaN / divergence abort in `alpha_rl_train.rs`.** A NaN-poisoned - run would burn the full 14400-second `activeDeadlineSeconds` budget - silently. +### Code quality +- `feedback_no_stubs` — wire it for real or delete +- `feedback_no_todo_fixme` — no TODO/FIXME/XXX markers +- `feedback_no_hiding` — never suppress with `_` prefix or `#[allow]` +- `feedback_no_legacy_aliases` — no deprecated wrappers +- `feedback_no_feature_flags` — no `enable_*` / `use_*` booleans +- `feedback_no_quickfixes` — every issue gets the proper fix +- `feedback_no_functionality_removal` — never delete features; fix what's broken +- `feedback_fix_everything_aggressively` — zero tolerance, no deferrals +- `feedback_extending_existing_code_audits_for_existing_violations` — when extending existing code, audit it FIRST against current rules -### Pattern lesson +### Architecture discipline +- `feedback_single_source_of_truth_no_duplicates` — no `v2_` / parallel impls +- `feedback_no_partial_refactor` — contract change = every consumer migrates atomically +- `feedback_wire_everything_up` — every kernel/module wired in same commit; no orphans +- `feedback_isv_for_adaptive_bounds` — adaptive bounds live in ISV, not hardcoded constants +- `feedback_adaptive_not_tuned` — signal-driven (EMA/windowed), not tuned constants +- `feedback_magnitude_must_be_useful` — DQN magnitude branch never deleted -The convergence-gate fixtures verified the gradient mechanics -(`reward_signal_drives_argmax_to_rewarded_action`) but their -state-invariant fixture intrinsically *can't* test: -- Sparse-reward credit assignment (defect #1's γ = 0 effect) -- Reward-magnitude drift (defect #2) -- Trust-region clipping (defect #1's ε = 0 effect) -- Target-network staleness (defect #4) +### Training discipline +- `feedback_always_per` — PER always enabled; non-PER paths are dead code +- `feedback_mbp10_mandatory` — `--mbp10-data-dir` mandatory; trades-data-dir paired for OFI completeness +- `feedback_default_to_l40s_pool` — defaults to `ci-training-l40s`, not H100 +- `feedback_stop_on_anomaly` — terminate on training anomaly, diagnose, fix, re-run +- `feedback_kill_runs_on_anomaly_quickly` — kill smoke at first useful signal -**Pre-cluster falsifiability requires fixtures that exercise the same -failure modes the cluster will see.** A short bull-market loop with -state-dependent rewards + per-step holds + multi-step trades is the -minimum. +### Workflow +- `feedback_push_before_deploy` — always `git push` before `argo submit` +- `feedback_argo_template_must_apply` — `kubectl apply -f