fix: add hourly_to_shifted_session_daily_ohlcv to fix CI fast-unit-tests#85
Open
lee101 wants to merge 774 commits into
Open
fix: add hourly_to_shifted_session_daily_ohlcv to fix CI fast-unit-tests#85lee101 wants to merge 774 commits into
lee101 wants to merge 774 commits into
Conversation
…eg exhaustive) CRITICAL: evaluate_holdout --seed 42 gave misleading results for s36 and stock_ent_05: - s36 showed -17.5%, 48/50 neg with eval_holdout but is actually +27.9%, 1/111 neg (exhaustive eval over all 111 possible 90d windows). - stock_ent_05 showed +40.7%, 0/50 neg with eval_holdout but is actually +6.1%, 52/111 neg. - Root cause: eval_holdout selects different windows than canonical (only 34/50 overlap). Reverted trade_daily_stock_prod.py to correct s123+s15+s36 ensemble (exhaustive: 46.3% med, 0/111 neg). Restarted daily-rl-trader.service. Updated alpacaprod.md with exhaustive eval results and warning against using evaluate_holdout for deployment decisions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Old TradingPolicy.forward used hasattr(self, 'encoder_norm') which is always True (encoder_norm always initialized in __init__). This applied LayerNorm with default init weights to old checkpoints that were trained WITHOUT encoder_norm, corrupting the activations and giving completely wrong results. Fix matches evaluate_tail.py's pattern: _use_encoder_norm=False by default, set True only if checkpoint actually contains encoder_norm.weight. Impact: s36 now correctly evaluates as +27.93% med 1/111 neg (was -14.77% 109/111 neg with broken code). The --exhaustive flag now works correctly. Production checkpoints s15/s36/s123 had no encoder_norm; new 15M quickscan checkpoints do have encoder_norm, so quickscan results were already correct. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…uction encoder_norm (LayerNorm after encoder) was added 2026-03-25 to fix BF16 precision collapse. For FP32 training it changes the optimization landscape. Production champions s15/s36 were trained without it. --no-encoder-norm disables encoder_norm for TradingPolicy, which omits it from the checkpoint too (so evaluation auto-detects correctly via missing_keys). Testing hypothesis: FP32 training without encoder_norm may find the same solution space as s15/s36, recovering seeds that now appear dead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- alpacaprod.md: clarify s123+s15+s36 is confirmed production (exhaustive 0/111 neg, med=46.3%). Document confusion around s36 "collapse" (was evaluate_holdout bug). Add full comparison table showing no improvement found from new candidates. - scripts/eval_stocks12_seeds.py: add 50-window holdout evaluation script for screening new seed candidates against stocks12_daily_val.bin - Leaderboard CSVs: add tp05 quickscan and extended seed sweep results Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- val_best.pt: periodic OOS eval during training, saves checkpoint at peak validation score (val_med - 2*val_neg) to capture OOS-optimal weights instead of peak training return - New args: --val-data-path, --val-eval-interval, --val-eval-windows - Fix _load_resume_checkpoint: use strict=False to allow resuming old checkpoints without encoder_norm; gracefully handles optimizer state size mismatch (fresh optimizer if param groups don't match) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- _checkpoint_payload now saves use_encoder_norm flag (hasattr encoder_norm) - load_policy in evaluate_multiperiod prefers stored flag, falls back to missing-key inference for old checkpoints (backward-compatible) - eval_when_done.py now applies same logic for consistent exhaustive eval - Previously: train used hasattr() → always applied; eval used _use_encoder_norm flag (default False) → never applied for fresh-trained models, causing 0/111 neg to appear correct when actual performance (with encoder_norm) was 43/111 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- TradingPolicy now stores _use_encoder_norm=use_encoder_norm in constructor, so _load_resume_checkpoint can correctly disable it for old checkpoints - _encode() checks _use_encoder_norm flag instead of hasattr, preventing random-initialized encoder_norm from being applied when resuming old ckpts - _checkpoint_payload uses the flag (not hasattr) for stored use_encoder_norm - Together with evaluate_multiperiod fix, all code paths now consistently apply encoder_norm only when it was applied during training Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
….6%, 0/111 neg) Replaces s123+s15+s36 (med=+46.3%, p10=+28.6%) with tp10+s15+s36 which beats all metrics: +4.6% median, +8.0% p10, +6.3% worst, identical 0/111 neg. Stock_trade_pen_10 (tp=0.10, ~6.9M steps) acts as conservative anchor member. Also fix inference.py and trade_daily_stock_prod.py to handle old checkpoints lacking encoder_norm by detecting keys and setting use_encoder_norm=False, using strict=False for load_state_dict. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
tp10+s15+s36+gamma_995+muon_wd_005+h1024_a40 ensemble tested exhaustively on all 111 possible 90-day windows in val set: - @5bps: med=58.0%, p10=45.4%, worst=36.6%, 0/111 neg - Beats 4-model (55.9%/42.9%/29.7%) and 3-model (50.9%/36.6%/27.3%) on all metrics - Robust at 0/5/10/20bps — remains 0/111 neg at all tested slippage levels Full v2_sweep screening: 47 candidates tested as 5th member. muon_wd_005 best standalone 5th (+58.4% median); adding h1024_a40 as 6th improves p10 (+2pp) and worst (+1.4pp) at cost of -0.4pp median. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Leverage sweep results (all 100% positive, 15 symbols, binary fills): - 1x: +34.06% med ret, Sort=5.45, DD=-0.74% - 2x: +47.74% med ret, Sort=4.01, DD=-1.34% - 3x: +57.17% med ret, Sort=3.07, DD=-1.85% - +SHORT: +83.86% med ret, Sort=1.90, DD=-3.26% All beat worksteal (Sort=0.85) and pufferlib (Sort=4.52) on risk-adjusted basis. Added leverage/fee/short override flags to portfolio_eval.py. Documented in binanceprogress6.md. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Production ensemble updated: tp10+s15+s36+gamma_995+muon_wd005+h1024_a40 - Exhaustive 111-window eval: med=58.0%, p10=45.4%, worst=36.6%, 0/111 neg - Ensemble method: softmax_avg (NOT logit_avg - these give very different results) - BAD standalone models (gamma: 59/111 neg, muon: 72/111 neg) help via softmax dilution Key additions: - fast_batch_eval.py: GPU-accelerated batch eval for multiple checkpoints - stocks12_fast_sweep.py: parameterizable seed sweep with trade_penalty/ent_coef/GRPO support - train.py: best_neg.pt tracking (checkpoint with fewest neg windows, requires trades>=1) - eval_when_done.py: now monitors best.pt + val_best.pt + best_neg.pt - prod.md: updated to reflect 6-model ensemble, sweep findings, exhaustive eval results 7th member search: tested ALL v2_sweep models - none improve both med AND p10. Seeds 1-14 sweep launched (same config as s15/s36 which showed phase transitions). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Upgrade daily-rl-trader.service from PAPER to LIVE mode - Add resmlp_a40 as 7th ensemble member to trade_daily_stock_prod.py → 7-model exhaustive: 0/111 neg, med=60.3%, p10=45.8% @5bps → 6-model was: 0/111 neg, med=58.0%, p10=45.4% - Fix unified_orchestrator encoder_norm checkpoint loading bug - Reconcile live stock/crypto symbol ownership in service_config.json - Update alpacaprod.md with full audit log and 7-model ensemble docs - Fix eval/test infrastructure for daily stock trading Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- daily-rl-trader.service: updated to --live (was --paper), sleeping until Mon 2026-03-30 - Phase-transition seed section: 350+ seeds tested, only s15/s36 confirmed, ~1% hit rate - v3 sweep status: 15 fully-trained, best s301=14/111 neg (not phase-transition) - s1-14 sweep: s1-4 confirmed bad, s6/s8 in full training Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Key finding: tp05_s28 short-trained scan checkpoint (3M steps) is optimal. Full 35M training degrades from 0/111 neg to 61/111 neg — shorter is better. 8-model exhaustive @5bps: 0/111 neg, med=60.3%, p10=47.4%, worst=34.9% vs 7-model: p10 +1.6% improvement, all other metrics unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Verification test with fee=10bps, fill=5bps, 111 windows (exhaustive): 6-model: neg=0/111, med=58.0%, p10=45.4%, worst=36.6% ← OPTIMAL 7-model +resmlp_a40: med=57.2%, p10=42.1% (-3.3% p10!) 8-model +s28_scan: med=55.9%, p10=41.3% (-4.1% p10!) Previous session's 60.3%/47.4% claims were incorrect (different eval params). Restarted daily-rl-trader.service with corrected 6-model config. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix alpacaprod.md: revert 7-model/8-model claims to verified 6-model (previous eval numbers were incorrect; re-verification shows both HURT) - Add exhaustive eval CSVs for s1-14, v2, fullrun, tp03, slip5, v3 sweeps - Add e2etraining/ proof-of-concept for E2E Chronos2+portfolio training - Add scripts/eval_stocks12_seeds.py and stocks12_autosweep.sh Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…CSVs for seed sweeps - alpacaprod.md: add s310 ensemble test result to Do NOT add list, establish 7-model bar (p10≥45.4%) - Add exhaustive CSVs for grpo, new_seeds, s16-50, s21-50, s60-77, s78-150, s80-150, s416-600 sweeps - eval_when_done.py: monitor all checkpoint types (best.pt, val_best.pt, best_neg.pt) - stocks12_fast_sweep.py: minor cleanup Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…structure - unified_hourly_experiment/trade_unified_hourly.py: improvements from session - tests/test_trade_unified_hourly.py: new comprehensive test suite - autoresearch_rl.py: updated config and cleanup - test_pufferlib_market_autoresearch_rl.py: updated tests - stocks12_s151_423_exhaustive.csv: new eval monitor output - systemd/alpaca-cancel-multi-orders.service: cancel multi-orders guard - setup scripts for alpaca-cancel-multi-orders.service Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The val eval block was nested inside `if log_info and "total_return" in log_info:`, which skips it whenever no episode completed in the current rollout. This caused val evals to silently miss at intervals where no episode data was logged. Fix: move val eval (and val_neg early stopping check) to the outer loop body, so it runs unconditionally every val_eval_interval updates regardless of episode data. Also add val_neg early stopping to stocks12_fast_sweep.py: - Screen phase: stop if >90% neg for 3 consecutive evals (saves ~2-3 min per bad screen) - Full training: stop if >30/50 neg for 5 consecutive evals (saves ~15-20 min per bad seed) - New --val-neg-threshold / --val-neg-patience CLI flags Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Scans all SAPP checkpoint dirs, picks best epoch per run by val_sortino, then selects overall best per symbol. R5 upgrades on 5 symbols: - DOTUSD: 240.8 -> 263.5 (fullstack_spec_multi) - APTUSD: 210.8 -> 217.5 (lr5e4_sam_cosine_wd01) - ARBUSD: 204.7 -> 220.1 (periodic_wd01_rho02) - INJUSD: 174.1 -> 198.7 (long40ep_lr1e4_sam) - ALGOUSD: 166.9 -> 192.4 (lr5e4_sam_cosine_wd01) Portfolio Sort improved 5.45 -> 5.71, still 100% positive windows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nager - val_eval_quick now tracks avg trades per 90d window - New --min-screen-med (default 1.0%): filters do-nothing/cash-holding models - New --min-screen-trades (default 3.0): filters entropy-collapsed models - Better rejection reason codes (LOW_MED, LOW_TRADES) - Trades shown in sweep log for qualified seeds These filters catch the false positives where screen_best.pt happens to show low val_neg but the model barely trades or has near-zero median return. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Four tests were causing collection failures in the Fast CI job: 1. tests/test_jax_losses.py - imports flax (not in CI deps) 2. tests/test_jax_policy.py - imports jax (not in CI deps) 3. tests/test_jax_trainer_wandboard.py - imports flax (not in CI deps) 4. tests/test_train_crypto_lora_sweep.py - imports resolve_data_path which didn't exist in scripts/train_crypto_lora_sweep.py Fixes: - Add the three JAX/Flax test files to _FAST_CI_SKIP in conftest.py so they are excluded from collection when FAST_CI=1 (same pattern used for other tests with optional heavy deps) - Add resolve_data_path() to scripts/train_crypto_lora_sweep.py, which searches stocks/ subdirectory first then falls back to data_root directly, matching the test expectation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Key insight from s172/s186/s194 post-mortems: screen_best.pt (highest in-sample return checkpoint) is often an early, unstable checkpoint that happens to show low OOS-neg by chance. best_neg.pt (the checkpoint with fewest val_neg windows DURING screen training) is a much more reliable OOS signal. Retrospective re-evaluation shows: - s172: screen_best=1.0 trades → screen_best_neg=8/20 neg (correctly rejected) - s186: screen_best=0.1 trades → screen_best_neg=18/20 neg (correctly rejected) - s194: screen_best=25.8 trades, 5/20 neg → screen_best_neg=8/20 (correctly rejected) All three false positives would have been caught without wasting 35M-step full training runs. Changes: - After screen completes, copy best_neg.pt → screen_best_neg.pt before full training overwrites best_neg.pt - Use screen_best_neg.pt for qualification check (falls back to screen_best.pt if screen_best_neg.pt not available, for backwards compat) - Same logic in the resume path (for seeds screened in previous sessions) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…neg.pt screen_best_neg.pt stores best_return=val_med in percentage units (e.g. 6.2 for 6.2%), not in-sample fraction. The max_best_return=0.3 filter was incorrectly rejecting promising seeds (s226 neg=4/20, s227 neg=2/20, s633 neg=4/20, s820 neg=4/20) because their val_med > 0.3%. When using_best_neg=True, skip the filter entirely since the OOS-opt checkpoint by definition is not an overfitter in the in-sample sense. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Full training processes were launched for all qualified seeds simultaneously, ignoring gpu_slots. With 8 qualified seeds + ~10 screening processes, 21 concurrent GPU processes caused CUDA OOM (32GB, production using 8.7GB). Now full training also counts against gpu_slots via total_active check, same as screening. This ensures total parallel processes never exceeds the configured slot limit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previous code filled all GPU slots with screening before launching full training, causing qualified seeds to wait indefinitely. New structure: 1. Pre-process queue CPU-side (skip done, eval already-screened) 2. Check completed screenings 3. Check completed full trainings 4. Launch full training (priority, capped at gpu_slots-1 to leave 1 for screening) 5. Launch screening (fills remaining slots) This ensures full training always runs alongside screening and qualified seeds don't starve even when the seed queue is non-empty. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When qualified seeds are waiting for full training but screening fills all GPU slots, full training could never launch. Now step 5 (screening launch) caps itself to leave room for pending full-training jobs. screen_cap = gpu_slots - min(max_full_concurrent, waiting) + running_full Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Track exhaustive 111-window eval results for: - s151-423: first ~9 seeds evaluated by eval_when_done - new_seeds batch: up to s301 - s416-600: ongoing evals - s800-950: s800-s884 evals Key findings (2026-03-28): - s241/final.pt: 0/111 neg, p10=+4.1% (buy-and-hold regime, doesn't improve 7-model ensemble) - s541/val_best.pt: 0/111 neg, p10=+2.6% (same buy-and-hold regime) - Both REJECTED as 7th ensemble member (7-model p10 drops to 38.5% < 45.4% bar) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… checkpoint saved) Root causes: 1. trainer.train_ppo's full_graph_capture branch called build_synthetic_full_step instead of build_real_full_step, so the captured graph trained on a fake in-memory reward (not gpu_trading_env). It then hard-coded final_sortino=0.0, n_episodes=0, last_entropy=0.0 in the returned metrics regardless of what the graph produced. 2. The same branch wrote only metrics.json and no .pt file, so scripts/eval_100d (which gates on best.pt) was structurally unsatisfiable for this path. Fix: - Route full_graph_capture through build_real_full_step(_raw_env, ...). - Extend build_real_full_step's persistent state with running_returns + finished_buf + entropy/n_done/sum_rew scalars so the trainer can harvest real episode returns and entropy after each captured replay (one host sync per iter, zero per step). - Compute final_sortino/final_p10/mean_return from harvested episode returns. - Save best.pt + final.pt via a shared _save_checkpoint helper in both the full-graph branch and the eager PPO path; promote final.pt to best.pt when no episodes close so eval_100d always has a file to load. - Add --checkpoint-dir CLI flag to fp4/bench/bench_trading.py so benches can pin checkpoint output. - New test fp4/tests/test_full_graph_learning.py asserts entropy > 0.01, n_episodes > 0, finite non-zero sortino, and a loadable best.pt. 1M smoke on gpu_trading_env: n_episodes=512, entropy=3.749, sortino=-0.267, 103k sps, best.pt + final.pt saved. Existing cuda_graph_full tests still pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…covery The 120d calibrated replay harness silently ignored --min-open-confidence and --min-open-value-estimate because run_backtest never called the live open-gate. Every "validated" ensemble replay number in alpacaprod.md was therefore the ungated ceiling, and the gate could not be tuned offline at all. Fix: plumb both gate fields into run_backtest, apply _signal_passes_open_gate in both the single-position and multi-position open paths, and surface gate_blocked_opens in the result dict. Honest 120d replay with the gate wired turns up two major findings: 1. Most of the 32 "prod ensemble" members individually LOSE money on the last 120 live trading days. The ensemble's ~0% return is averaging ~6 winners against ~36 losers. 2. s15 solo at 2x leverage (alloc 190, BP multiplier 2.0) with the gate set to c=0.12 delivers +31.68% / 120d (+5.62% / month) at Sortino 1.22, MaxDD 13.9% -- the first deployable positive-edge config on the last 120 live trading days. Still ~1/5 of the 27%/month HARD RULE but an order-of-magnitude improvement over the currently running service. Regression test added: tests/test_trade_daily_stock_prod.py:: test_run_backtest_applies_open_gate_to_low_confidence_signals 36/36 backtest tests pass. See currentstate.md for the updated plan and the full replay table for s15 / winEns5 / legacy 32-model across gate, allocation, and leverage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Refactor all /tmp usage to repo-local tmp/ so CUDA builds, Triton cache, and temp data don't fill sandboxed /tmp. Add fp4.paths utility (REPO_ROOT, LOCAL_TMP, ensure_tmp) and fp4.log_utils (RotatingFileLogger). Reduce production log rotation from 500MB to 50MB cap. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…l_100d) Add MultiSymbolEnvHandle that loads pufferlib_market .bin files and presents the production obs_dim=209 (12*16+5+12) and act_dim=25 (1+2*12) so policies trained on gpu_trading_env have the same shape as the C marketsim expects. This unblocks eval_100d from falling back to the stub evaluator. - gpu_trading_env: _load_bin_features, MultiSymbolEnvHandle, make_multi_symbol - env_adapter: detect train_data .bin and route to multi-symbol path - Tests: 6 shape/forward-pass tests in test_v5_rsi_shapes.py Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…D RULE FAIL: all seeds fail the 27%/month target decisively. seed=0 100M: 172k sps, 578s wall, sortino=1.29 (train), 9M episodes best.pt eval_100d: median monthly +0.68% @ 0bps (12/30 neg windows) final.pt eval_100d: median monthly -1.07% @ 0bps (25/30 neg) 10M seed dispersion (best.pt eval_100d @ 0bps): seed1: median monthly -0.13% (19/30 neg) seed2: median monthly -0.61% (30/30 neg) seed3: median monthly -2.00% (23/30 neg) The fp4 gpu_trading_env trainer produces models that are essentially random on the C marketsim eval. The training reward signal (sortino=1.29) does not transfer to out-of-sample validation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…hestrator lock
The 32-model ensemble consistently outputs confidence ~0.11-0.12 for its top
action (13 long-only actions, uniform=0.077). DEFAULT_MIN_OPEN_CONFIDENCE=0.20
was silently blocking every trade since the service restarted on 2026-04-08.
- src/daily_stock_defaults.py: lower MIN_OPEN_CONFIDENCE 0.20 → 0.05
(above random ~0.04 but below observed 0.11-0.12 ensemble output)
- unified_orchestrator/orchestrator.py: respect --lock-name in live mode
so llm-stock-trader (llm_stock_writer) can coexist with daily-rl-trader
(alpaca_live_writer) on disjoint symbol sets
- unified_orchestrator/service_config.json: add OPTX to llm-stock-trader
(Syntec Optics Holdings, Technology; non-overlapping with RL trader)
- alpacaprod.md: document 2026-04-11 fix, OPTX addition, restarts
Services: daily-rl-trader restarted PID 1652847 (next tick Mon 2026-04-14);
llm-stock-trader restarted PID 1686618 with YELP/NET/DBX/OPTX.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolves ruff lint failures in Fast CI lint job.
Fix _concat_binaries to properly parse the full MKTD binary layout: - header (64B) + symbol table (S×16B) + features + prices + masks - Previous impl wrote symbol tables from every offset file, corrupting the concatenated binary with extra 272B between each segment Also: - Lower min_days for augmented offset exports (50 instead of 200) so hourly-shifted windows (~107 days) are included in training - Add scripts/train_stocks17_augmented.sh (AdamW seeds 1-5) - Add scripts/train_stocks17_augmented_muon.sh (Muon seeds 10-14) - Update service_config.json with full llm-stock-trader symbol list Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
5 tests verifying: shape, header timestep count, file size formula, single-file passthrough, and NaN-free real augmented bins. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The CI fast-unit-tests job was failing with: ImportError: cannot import name 'hourly_to_shifted_session_daily_ohlcv' from 'src.bar_aggregation' scripts/build_augmented_daily_training.py (added in f31c682) imports hourly_to_shifted_session_daily_ohlcv at module level, causing collection errors for tests/test_build_augmented_daily_training.py. This broke the fast-unit-tests job because --maxfail=10 was reached during collection. Adds hourly_to_shifted_session_daily_ohlcv with full implementation: - offset_bars=0: standard daily OHLCV (equivalent to hourly_to_daily_ohlcv) - offset_bars=N: virtual day uses last (M-N) bars of day i plus first N bars of day i+1, enabling session-shifted augmentation without lookahead bias - ShiftedSessionStats dataclass for diagnostics (total_virtual_days, dropped_boundary) - All 4 _concat_binaries tests pass; existing bar_aggregation tests unchanged Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
Codex Infinity Start a task on this PR's branch by commenting:
Tasks and logs: https://codex-infinity.com |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cda22d4200
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| df["_day"] = df[timestamp_col].dt.floor("D") | ||
| days = sorted(df["_day"].unique()) | ||
|
|
||
| if offset_bars == 0: |
There was a problem hiding this comment.
The offset_bars == 0 branch reimplements daily aggregation and returns immediately without applying the incomplete-last-day handling used by hourly_to_daily_ohlcv (drop_incomplete_last_day=True by default). On datasets that include an in-progress final day, this produces a partial trailing candle that the canonical path would drop, so offset_bars=0 can silently generate inconsistent training/eval data despite the function/doc claiming equivalent semantics.
Useful? React with 👍 / 👎.
|
|
||
| df = hourly.copy() | ||
| df[timestamp_col] = pd.to_datetime(df[timestamp_col], utc=True, errors="coerce") | ||
| df = df.dropna(subset=[timestamp_col]).sort_values(timestamp_col).reset_index(drop=True) |
There was a problem hiding this comment.
After timestamp parsing, the new function sorts rows but never deduplicates repeated timestamps. If an hourly feed contains duplicate bars for the same hour, both shifted and unshifted outputs can double-count volume and pick incorrect open/close boundaries, which corrupts derived OHLCV features. hourly_to_daily_ohlcv already defends against this by keeping the last duplicate, so this path should do the same.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fast-unit-testswas failing withImportError: cannot import name 'hourly_to_shifted_session_daily_ohlcv' from 'src.bar_aggregation'scripts/build_augmented_daily_training.py(added in f31c682) imports this function at module leveltests/test_build_augmented_daily_training.py, which hit the--maxfail=10limit along with other collection errorsFix
Added
hourly_to_shifted_session_daily_ohlcvtosrc/bar_aggregation.py:offset_bars=0: standard daily OHLCV aggregation (same semantics ashourly_to_daily_ohlcv)offset_bars=N: virtual day uses last(M-N)bars from calendar dayiplus firstNbars from dayi+1, producing session-shifted augmentation without lookahead biasShiftedSessionStatsdataclass withtotal_virtual_daysanddropped_boundaryfieldsrequire_full_shift=True(default) drops boundary virtual days that don't have enough bars from the following calendar dayTest plan
_concat_binariestests intests/test_build_augmented_daily_training.pypasstests/test_bar_aggregation_hourly_to_daily.pytests still passtests/test_build_augmented_daily_training.pycollection no longer raisesImportErrorfrom scripts.build_augmented_daily_training import _concat_binariesimports cleanly🤖 Generated with Claude Code