Skip to content

Fix TopkDropoutStrategy dynamic risk sizing#2281

Open
Kevin-Li-2025 wants to merge 1 commit into
microsoft:mainfrom
Kevin-Li-2025:kevin/topk-dropout-dynamic-risk
Open

Fix TopkDropoutStrategy dynamic risk sizing#2281
Kevin-Li-2025 wants to merge 1 commit into
microsoft:mainfrom
Kevin-Li-2025:kevin/topk-dropout-dynamic-risk

Conversation

@Kevin-Li-2025

Copy link
Copy Markdown

Summary

Fixes #2276.

TopkDropoutStrategy exposes get_risk_degree(trade_step) so subclasses can implement dynamic market-timing / position-sizing logic, but the buy budget used the fixed self.risk_degree field directly. This makes dynamic risk overrides ineffective for new buys.

This PR routes the buy budget through get_risk_degree(trade_step) and adds a focused regression test that keeps self.risk_degree = 0.95 while overriding get_risk_degree() to return 0.25; the generated buy order must use the dynamic 25% budget.

Tests

  • PYTHONPYCACHEPREFIX=/private/tmp/qlib-pycache /usr/bin/python3 -m py_compile qlib/contrib/strategy/signal_strategy.py tests/backtest/test_topk_dropout_strategy.py
  • git diff --check
  • PYTHONPYCACHEPREFIX=/private/tmp/qlib-pycache /opt/homebrew/bin/python3.11 - <<'PY' ... isolated harness that stubs Qlib package-level imports, loads qlib/contrib/strategy/signal_strategy.py, and verifies dynamic risk produces a 25-share order from 1000 cash at price 10

Note: direct local pytest -q tests/backtest/test_topk_dropout_strategy.py is blocked in this unbuilt source tree by missing compiled extensions (qlib.data._libs.rolling). I attempted setup.py build_ext --inplace; this checkout lacks generated C++ sources and needs the project Cython build path first. The added test is a normal repository test and should run in CI after the extension build step.

@Kevin-Li-2025

Copy link
Copy Markdown
Author

Quick follow-up on scope: this is intentionally a small fix for #2276. TopkDropoutStrategy already exposes get_risk_degree(trade_step) for dynamic risk / market timing, and WeightStrategyBase already routes sizing through that hook. This PR makes the TopK dropout buy budget use the same hook instead of the fixed self.risk_degree field.

I added a focused regression test where self.risk_degree remains 0.95 but an override returns 0.25, and the generated buy order is sized from the dynamic 25% budget. Happy to adjust the test style if maintainers prefer this covered in an existing backtest test file.

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.

TopkDropoutStrategy ignores get_risk_degree() for position sizing

1 participant