fix(architectural): include bar formation params in fxcache key + actually USE imbalance bars
## The bug (audit 2026-05-09) `crates/ml/src/feature_cache.rs:calculate_dbn_cache_key_full` hashed only `(symbol, data_source, dbn_filenames+sizes)` — NOT `imbalance_bar_threshold` or `imbalance_bar_ewma_alpha`. Combined with `precompute_features.rs:346` unconditionally calling `build_volume_bars` regardless of `data_source`, this meant: 1. 14 audited production runs (Apr 11–May 8) all collided on the same fxcache key (`a3f933aa...` / `c07c960a...`) regardless of TOML `imbalance_bar_threshold` value 2. The imbalance-bar code path was reachable only via fxcache MISS, which never happens in production because `ensure-fxcache` always populates first 3. Every "tuning" of `imbalance_bar_threshold` across 16+ SP runs was a silent no-op — the system was actually running volume bars at DEFAULT_VOLUME_BAR_SIZE (100 contracts/bar) ## The fix (this commit) **Part A — cache key includes bar formation params:** - `calculate_dbn_cache_key_full` signature: 5 args → 7 args. Two new f64 params hashed via `to_le_bytes()`. - 4 callers updated atomically (per `feedback_no_partial_refactor`). - 2 new unit tests (`test_cache_key_includes_bar_threshold`, `test_cache_key_includes_bar_alpha`) pin the contract. **Part B — precompute_features actually USES data_source:** - New CLI args `--imbalance-bar-threshold` (default 0.5) and `--imbalance-bar-ewma-alpha` (default 0.1) on both train_baseline_rl and precompute_features. - `precompute_features.rs:346` now branches: when `data_source == "mbp10"` AND `mbp10_data_dir.is_some()`, calls `mbp10_to_imbalance_bars` instead of `build_volume_bars`. **Argo plumbing:** - `train-template.yaml` + `train-multi-seed-template.yaml`: new workflow parameters threaded into BOTH precompute and trainer invocations so both compute the same fxcache key. - `scripts/argo-train.sh`: new CLI flags for ad-hoc overrides. - ensure-fxcache regen path: removed `rm -f /feature-cache/*.fxcache` (with bar-params now in key, parallel experiments coexist). ## Effects going forward - Tuning `imbalance_bar_threshold` actually changes bar density - Configuring `data_source = "mbp10"` actually produces imbalance bars - Multiple parallel experiments at different thresholds coexist on PVC - `dqn-production.toml: imbalance_bar_threshold = 0.5` no longer ignored Default values match prior production behavior → existing wgdc7-equivalent runs reproduce, just with a *new* fxcache key (the old volume-bar cache file is still on disk but won't be hit; harmless, can GC manually). Audit-doc: `docs/dqn-wire-up-audit.md` updated with full context. Tests: 5/5 feature_cache tests pass, full workspace + examples compile clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -147,6 +147,20 @@ struct Opts {
|
||||
#[arg(long, default_value = "mbp10")]
|
||||
data_source: String,
|
||||
|
||||
/// Imbalance bar formation threshold. ONLY used when `data_source == "mbp10"`
|
||||
/// and `mbp10_data_dir` is set; otherwise volume bars are produced. MUST
|
||||
/// match the trainer's `imbalance_bar_threshold` for fxcache HIT (the cache
|
||||
/// key includes this value as of 2026-05-09 architectural fix). Default
|
||||
/// matches `dqn-production.toml: imbalance_bar_threshold = 0.5`.
|
||||
#[arg(long, default_value_t = 0.5)]
|
||||
imbalance_bar_threshold: f64,
|
||||
|
||||
/// Imbalance bar EWMA alpha (weight on OLD threshold in this codebase's
|
||||
/// reversed convention; α=1.0 disables adaptation). Default matches
|
||||
/// `dqn-production.toml: 0.1`. MUST match the trainer's value for cache HIT.
|
||||
#[arg(long, default_value_t = 0.1)]
|
||||
imbalance_bar_ewma_alpha: f64,
|
||||
|
||||
/// Skip confirmation prompt
|
||||
#[arg(long)]
|
||||
yes: bool,
|
||||
@@ -262,6 +276,8 @@ async fn main() -> Result<()> {
|
||||
trades_dir.as_deref(),
|
||||
&opts.symbol,
|
||||
&opts.data_source,
|
||||
opts.imbalance_bar_threshold,
|
||||
opts.imbalance_bar_ewma_alpha,
|
||||
).context("Failed to compute cache key")?;
|
||||
let early_check_path = output_dir.join(format!("{hex_key_early}.fxcache"));
|
||||
if early_check_path.exists() {
|
||||
@@ -342,17 +358,47 @@ async fn main() -> Result<()> {
|
||||
all_front_month_trades.sort_by_key(|t| t.timestamp);
|
||||
info!("Total trades: {} raw, {} front-month", total_raw, all_front_month_trades.len());
|
||||
|
||||
// Build volume bars
|
||||
let all_bars = ml::features::build_volume_bars(&all_front_month_trades, ml::features::DEFAULT_VOLUME_BAR_SIZE);
|
||||
info!("Volume bars built: {} bars ({} contracts/bar) in {:.1}s",
|
||||
all_bars.len(), ml::features::DEFAULT_VOLUME_BAR_SIZE, t1.elapsed().as_secs_f64());
|
||||
// Build bars — branch on data_source. Pre-2026-05-09 this was unconditional
|
||||
// build_volume_bars, silently ignoring data_source="mbp10" + the imbalance
|
||||
// bar threshold config. Audit confirmed 14 production runs all collided on
|
||||
// the same fxcache key regardless of TOML threshold value, so the imbalance
|
||||
// path was effectively dead. Wire it now: when "mbp10" + mbp10_dir is set,
|
||||
// construct imbalance bars from MBP-10 trades; otherwise fall back to volume
|
||||
// bars on the raw trade tape.
|
||||
let all_bars = if opts.data_source == "mbp10" && mbp10_dir.is_some() {
|
||||
let mbp10_path = mbp10_dir.as_deref().expect("mbp10_dir checked above");
|
||||
info!(
|
||||
"Building imbalance bars from MBP-10: dir={}, threshold={}, ewma_alpha={}",
|
||||
mbp10_path.display(),
|
||||
opts.imbalance_bar_threshold,
|
||||
opts.imbalance_bar_ewma_alpha,
|
||||
);
|
||||
let bars = ml::features::mbp10_loader::mbp10_to_imbalance_bars(
|
||||
mbp10_path,
|
||||
&opts.symbol,
|
||||
opts.imbalance_bar_threshold,
|
||||
opts.imbalance_bar_ewma_alpha,
|
||||
).context("MBP-10 imbalance bar construction failed")?;
|
||||
info!(
|
||||
"Imbalance bars built: {} bars (threshold={}) in {:.1}s",
|
||||
bars.len(),
|
||||
opts.imbalance_bar_threshold,
|
||||
t1.elapsed().as_secs_f64(),
|
||||
);
|
||||
bars
|
||||
} else {
|
||||
let bars = ml::features::build_volume_bars(&all_front_month_trades, ml::features::DEFAULT_VOLUME_BAR_SIZE);
|
||||
info!("Volume bars built: {} bars ({} contracts/bar) in {:.1}s",
|
||||
bars.len(), ml::features::DEFAULT_VOLUME_BAR_SIZE, t1.elapsed().as_secs_f64());
|
||||
bars
|
||||
};
|
||||
|
||||
// SP19 Path (B) (2026-05-09): need at least `WARMUP + LOOKAHEAD_HORIZON_MAX + 1`
|
||||
// bars so the 30-bar log-return at `i = 0` (`all_bars[WARMUP + 30]`) is
|
||||
// valid AND we still produce at least one output row.
|
||||
if all_bars.len() < WARMUP + LOOKAHEAD_HORIZON_MAX + 1 {
|
||||
anyhow::bail!(
|
||||
"Insufficient data: {} volume bars, need at least {} (WARMUP={WARMUP} + LOOKAHEAD_HORIZON_MAX={LOOKAHEAD_HORIZON_MAX} + 1)",
|
||||
"Insufficient data: {} bars, need at least {} (WARMUP={WARMUP} + LOOKAHEAD_HORIZON_MAX={LOOKAHEAD_HORIZON_MAX} + 1)",
|
||||
all_bars.len(), WARMUP + LOOKAHEAD_HORIZON_MAX + 1
|
||||
);
|
||||
}
|
||||
@@ -723,6 +769,8 @@ async fn main() -> Result<()> {
|
||||
trades_dir.as_deref(),
|
||||
&opts.symbol,
|
||||
&opts.data_source,
|
||||
opts.imbalance_bar_threshold,
|
||||
opts.imbalance_bar_ewma_alpha,
|
||||
).context("Failed to compute cache key")?;
|
||||
let cache_key: [u8; 32] = hex::decode(&hex_key)
|
||||
.context("Invalid hex key")?
|
||||
|
||||
@@ -237,6 +237,21 @@ struct Args {
|
||||
#[arg(long)]
|
||||
trades_data_dir: Option<PathBuf>,
|
||||
|
||||
/// Imbalance bar formation threshold (signed buy/sell volume that triggers
|
||||
/// a bar close). Must match the value used by `precompute_features` to
|
||||
/// populate the fxcache, otherwise this trainer's discover_and_load call
|
||||
/// MISSES and falls back to in-process MBP-10 loading. Default matches
|
||||
/// `dqn-production.toml: imbalance_bar_threshold = 0.5`.
|
||||
#[arg(long, default_value_t = 0.5)]
|
||||
imbalance_bar_threshold: f64,
|
||||
|
||||
/// Imbalance bar EWMA alpha. Convention in this codebase: weight on the
|
||||
/// OLD threshold in `T_new = α·T_old + (1-α)·observed`. So α=1.0 disables
|
||||
/// adaptation. Default matches `dqn-production.toml: 0.1`. Must match the
|
||||
/// value used by `precompute_features` for fxcache hit.
|
||||
#[arg(long, default_value_t = 0.1)]
|
||||
imbalance_bar_ewma_alpha: f64,
|
||||
|
||||
/// Enable offline RL mode: train exclusively from a pre-collected dataset,
|
||||
/// skipping online experience collection. Requires --dataset-path.
|
||||
#[arg(long, default_value_t = false)]
|
||||
@@ -602,6 +617,8 @@ fn run_training(args: &Args) -> Result<Vec<RlTrainingResult>> {
|
||||
mbp10.map(|p| p.as_path()),
|
||||
trades.map(|p| p.as_path()),
|
||||
"mbp10",
|
||||
args.imbalance_bar_threshold,
|
||||
args.imbalance_bar_ewma_alpha,
|
||||
cache_dir_override.as_deref(),
|
||||
);
|
||||
|
||||
|
||||
@@ -13,20 +13,33 @@ use std::path::{Path, PathBuf};
|
||||
/// Hashes every `.dbn` and `.dbn.zst` file found recursively under `data_dir`
|
||||
/// by (canonical path, size, mtime). Any change to the directory contents
|
||||
/// (add / remove / modify) produces a different key.
|
||||
///
|
||||
/// Production defaults for the bar-formation params (matches `dqn-production.toml`):
|
||||
/// `imbalance_bar_threshold = 0.5`, `imbalance_bar_ewma_alpha = 0.1`.
|
||||
pub fn calculate_dbn_cache_key(data_dir: &Path) -> Result<String> {
|
||||
calculate_dbn_cache_key_full(data_dir, None, None, "ES.FUT", "mbp10")
|
||||
calculate_dbn_cache_key_full(data_dir, None, None, "ES.FUT", "mbp10", 0.5, 0.1)
|
||||
}
|
||||
|
||||
/// Extended cache key that includes MBP-10 and trades directories.
|
||||
/// The cache is invalidated when ANY data source changes — OHLCV, MBP-10, or trades.
|
||||
/// Without this, a cache built without trades would serve stale features
|
||||
/// even after trades data is added, silently dropping VPIN/Kyle's Lambda enrichment.
|
||||
/// Extended cache key that includes MBP-10 and trades directories AND bar
|
||||
/// formation parameters. The cache is invalidated when ANY data source
|
||||
/// changes — OHLCV, MBP-10, trades — OR when bar-formation params change.
|
||||
///
|
||||
/// Bar-formation params (`imbalance_bar_threshold`, `imbalance_bar_ewma_alpha`)
|
||||
/// are hashed in regardless of `data_source` because: (a) they uniquely identify
|
||||
/// the bar stream the cache contains, (b) downstream features (e.g. log-returns,
|
||||
/// targets) depend on bar boundaries, and (c) without this, two runs configured
|
||||
/// with different thresholds will silently share the same cache file (canonical
|
||||
/// 2026-05-09 audit: 14 prior production runs all collided on `a3f933aa...` /
|
||||
/// `c07c960a...` keys regardless of TOML threshold value because the bar params
|
||||
/// were not in the hash).
|
||||
pub fn calculate_dbn_cache_key_full(
|
||||
data_dir: &Path,
|
||||
mbp10_dir: Option<&Path>,
|
||||
trades_dir: Option<&Path>,
|
||||
symbol: &str,
|
||||
data_source: &str,
|
||||
imbalance_bar_threshold: f64,
|
||||
imbalance_bar_ewma_alpha: f64,
|
||||
) -> Result<String> {
|
||||
let mut hasher = Sha256::new();
|
||||
|
||||
@@ -36,6 +49,12 @@ pub fn calculate_dbn_cache_key_full(
|
||||
hasher.update(data_source.as_bytes());
|
||||
hasher.update(b"|");
|
||||
|
||||
// Include bar-formation params — different bar streams get different keys
|
||||
hasher.update(&imbalance_bar_threshold.to_le_bytes());
|
||||
hasher.update(b"|");
|
||||
hasher.update(&imbalance_bar_ewma_alpha.to_le_bytes());
|
||||
hasher.update(b"|");
|
||||
|
||||
// Hash only file contents (size + mtime), not paths.
|
||||
// This makes the key independent of CWD, relative/absolute paths,
|
||||
// and directory naming — only actual data changes invalidate it.
|
||||
@@ -82,8 +101,8 @@ mod tests {
|
||||
fn test_cache_key_includes_symbol() {
|
||||
let dir = Path::new("test_data/futures-baseline");
|
||||
if !dir.exists() { return; }
|
||||
let key_es = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10").unwrap();
|
||||
let key_nq = calculate_dbn_cache_key_full(dir, None, None, "NQ.FUT", "mbp10").unwrap();
|
||||
let key_es = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10", 0.5, 0.1).unwrap();
|
||||
let key_nq = calculate_dbn_cache_key_full(dir, None, None, "NQ.FUT", "mbp10", 0.5, 0.1).unwrap();
|
||||
assert_ne!(key_es, key_nq, "Different symbols must produce different cache keys");
|
||||
}
|
||||
|
||||
@@ -91,17 +110,35 @@ mod tests {
|
||||
fn test_cache_key_includes_data_source() {
|
||||
let dir = Path::new("test_data/futures-baseline");
|
||||
if !dir.exists() { return; }
|
||||
let key_ohlcv = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10").unwrap();
|
||||
let key_mbp10 = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10").unwrap();
|
||||
let key_ohlcv = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "ohlcv", 0.5, 0.1).unwrap();
|
||||
let key_mbp10 = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10", 0.5, 0.1).unwrap();
|
||||
assert_ne!(key_ohlcv, key_mbp10, "Different data sources must produce different cache keys");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_cache_key_includes_bar_threshold() {
|
||||
let dir = Path::new("test_data/futures-baseline");
|
||||
if !dir.exists() { return; }
|
||||
let key_a = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10", 0.5, 0.1).unwrap();
|
||||
let key_b = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10", 2.5, 0.1).unwrap();
|
||||
assert_ne!(key_a, key_b, "Different imbalance_bar_threshold must produce different cache keys");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_cache_key_includes_bar_alpha() {
|
||||
let dir = Path::new("test_data/futures-baseline");
|
||||
if !dir.exists() { return; }
|
||||
let key_a = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10", 0.5, 0.1).unwrap();
|
||||
let key_b = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10", 0.5, 1.0).unwrap();
|
||||
assert_ne!(key_a, key_b, "Different imbalance_bar_ewma_alpha must produce different cache keys");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_cache_key_stable() {
|
||||
let dir = Path::new("test_data/futures-baseline");
|
||||
if !dir.exists() { return; }
|
||||
let key1 = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10").unwrap();
|
||||
let key2 = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10").unwrap();
|
||||
let key1 = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10", 0.5, 0.1).unwrap();
|
||||
let key2 = calculate_dbn_cache_key_full(dir, None, None, "ES.FUT", "mbp10", 0.5, 0.1).unwrap();
|
||||
assert_eq!(key1, key2, "Same inputs must produce same key");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -689,7 +689,9 @@ pub fn norm_stats_path_for_key(
|
||||
/// * `symbol` — Trading symbol (e.g. `"ES.FUT"`)
|
||||
/// * `mbp10_dir` — Optional MBP-10 order book data directory
|
||||
/// * `trades_dir` — Optional trades data directory
|
||||
/// * `data_source` — Data source mode (`"mbp10"` or `"mbp10"`)
|
||||
/// * `data_source` — Data source mode (`"mbp10"` or `"ohlcv"`)
|
||||
/// * `imbalance_bar_threshold` — Bar formation threshold (must match producer's value)
|
||||
/// * `imbalance_bar_ewma_alpha` — Bar formation EWMA alpha (must match producer's value)
|
||||
/// * `cache_dir_override` — Explicit cache directory (from CLI `--feature-cache-dir`)
|
||||
pub fn discover_and_load(
|
||||
data_dir: &Path,
|
||||
@@ -697,14 +699,17 @@ pub fn discover_and_load(
|
||||
mbp10_dir: Option<&Path>,
|
||||
trades_dir: Option<&Path>,
|
||||
data_source: &str,
|
||||
imbalance_bar_threshold: f64,
|
||||
imbalance_bar_ewma_alpha: f64,
|
||||
cache_dir_override: Option<&Path>,
|
||||
) -> Option<FxCacheData> {
|
||||
// 1. Resolve cache directory
|
||||
let cache_dir = resolve_cache_dir(data_dir, cache_dir_override)?;
|
||||
|
||||
// 2. Compute cache key (includes symbol + data_source)
|
||||
// 2. Compute cache key (includes symbol + data_source + bar params)
|
||||
let key_hex = crate::feature_cache::calculate_dbn_cache_key_full(
|
||||
data_dir, mbp10_dir, trades_dir, symbol, data_source,
|
||||
imbalance_bar_threshold, imbalance_bar_ewma_alpha,
|
||||
)
|
||||
.ok()?;
|
||||
let key: [u8; 32] = hex::decode(&key_hex).ok()?.try_into().ok()?;
|
||||
@@ -742,6 +747,8 @@ mod discover_tests {
|
||||
None,
|
||||
None,
|
||||
"mbp10",
|
||||
0.5,
|
||||
0.1,
|
||||
None,
|
||||
);
|
||||
assert!(result.is_none());
|
||||
@@ -771,6 +778,8 @@ mod discover_tests {
|
||||
None,
|
||||
None,
|
||||
"mbp10",
|
||||
0.5,
|
||||
0.1,
|
||||
Some(&cache_dir),
|
||||
);
|
||||
// Strict match only — no "most recent" fallback
|
||||
|
||||
@@ -149,6 +149,8 @@ impl DQNTrainer {
|
||||
mbp10_path,
|
||||
trades_path,
|
||||
&self.hyperparams.data_source,
|
||||
f64::from(self.hyperparams.imbalance_bar_threshold),
|
||||
f64::from(self.hyperparams.imbalance_bar_ewma_alpha),
|
||||
cache_dir_override,
|
||||
) {
|
||||
// OFI is unconditional — the model requires order flow data.
|
||||
|
||||
Reference in New Issue
Block a user