Skip to content

Fix vacuous IC test assertion; surface silent deal-price fallback; warn on processor fit-range leakage#2301

Open
SatishRockzz wants to merge 1 commit into
microsoft:mainfrom
SatishRockzz:audit-fixes
Open

Fix vacuous IC test assertion; surface silent deal-price fallback; warn on processor fit-range leakage#2301
SatishRockzz wants to merge 1 commit into
microsoft:mainfrom
SatishRockzz:audit-fixes

Conversation

@SatishRockzz

Copy link
Copy Markdown

Description

This PR fixes issues found during a systematic audit of the backtest engine, dataset processors, and test suite. Each fix was verified by execution before being applied; default behavior is preserved everywhere except where noted.

1. The pipeline-quality test could never fail (tests/test_all_pipeline.py)
self.assertGreaterEqual(ic_ric["ic"].all(), 0, ...)Series.all() returns a bool, and both True (1) and False (0) satisfy >= 0, so the assertion passes even if every IC is negative. Now asserts ic_ric["ic"].mean() > 0 (same for ric), verified against the actual ic.pkl Series structure.

2. Silent deal-price fallback made visible + opt-in reject mode (qlib/backtest/exchange.py)
When the configured deal price ($open/$vwap) is NaN or ≤1e-8 — typically a barely-trading stock — the engine silently substituted $close, changing execution semantics per-stock per-day. Substitutions are now counted (_deal_price_fallback_count) and logged with running totals, and a new constructor parameter deal_price_fallback="close"|"reject" lets users treat missing-price orders as untradable instead. Default remains "close" for backward compatibility; the docstring documents the realism concern.

3. Runtime leakage warning for processor fit ranges (qlib/data/dataset/__init__.py, processor.py)
The only guard preventing fit_end_time on ZScoreNorm/MinMaxNorm/RobustZScoreNorm from overlapping the test period was a code comment. DatasetH now checks each processor's fit_end_time against the start of non-train segments and logs a POTENTIAL DATA LEAKAGE warning on overlap (defensive against slices/None/malformed segments; silent on clean configs — the shipped benchmark configs are clean). Inverted fit ranges now raise ValueError.

4. Engineering hygiene

  • Mutable default dealt_order_amount: Dict = defaultdict(float) on Exchange.deal_order (shared process-wide) → None-guarded; same for DropCol/FilterCol processors.
  • Exchange init now warns when quote rows have $volume == 0 but non-NaN $close — forward-filled suspension data would otherwise be treated as fully tradable at stale prices.
  • Rolling docstring documents the min_periods=1 partial-window behavior (newly-listed-stock pitfall) with a masking recipe; no behavior change.

Motivation and Context

The audit found that qlib's core alignment logic is careful, but several safety properties were enforced by comments/conventions rather than code. These changes convert the highest-value conventions into runtime signals without changing default results.

How Has This Been Tested?

  • Behavioral spot-tests of both deal_price_fallback modes, the leakage warning (fires on leaking config, silent on clean), check_fit_range, and per-instance processor defaults, run against the edited modules with compiled Cython extensions.
  • Existing data-free tests pass: tests/misc, test_structured_cov_estimator, test_register_ops (14 passed). Data-dependent suites require downloaded market data and were not run locally.

🤖 Generated with Claude Code

…range leakage

Fixes from a systematic audit:
- tests/test_all_pipeline.py: assertGreaterEqual(ic.all(), 0) could never
  fail (.all() returns a bool; both 0 and 1 pass >= 0); now asserts
  ic.mean() > 0 (same for ric) so the pipeline-quality check is real
- backtest/exchange.py: deal-price fallback (missing $open/$vwap replaced
  with $close) is now counted and logged with running totals; new opt-in
  deal_price_fallback="reject" treats missing-price orders as untradable;
  mutable default dict removed from deal_order; Exchange init now warns
  when rows have zero volume but non-NaN close (forward-filled suspension
  data would otherwise be treated as tradable)
- data/dataset: new runtime POTENTIAL DATA LEAKAGE warning when any
  processor's fit_end_time overlaps a valid/test segment (the previous
  guard was only a comment); ValueError on inverted fit ranges in
  MinMax/ZScore/RobustZScore normalizers; DropCol/FilterCol mutable
  defaults fixed
- data/ops.py: Rolling docstring documents min_periods=1 partial-window
  values (newly-listed-stock pitfall) with a masking recipe

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@SatishRockzz

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant