Skip to content

fix: TopkDropoutStrategy honor overridden get_risk_degree() for buy sizing#2277

Open
zhaow-de wants to merge 1 commit into
microsoft:mainfrom
zhaow-de:fix/topk-dropout-get-risk-degree
Open

fix: TopkDropoutStrategy honor overridden get_risk_degree() for buy sizing#2277
zhaow-de wants to merge 1 commit into
microsoft:mainfrom
zhaow-de:fix/topk-dropout-get-risk-degree

Conversation

@zhaow-de

@zhaow-de zhaow-de commented Jun 21, 2026

Copy link
Copy Markdown

Description

TopkDropoutStrategy.generate_trade_decision sized new buys with the raw attribute self.risk_degree instead of the getter self.get_risk_degree(trade_step) (qlib/contrib/strategy/signal_strategy.py:266):

# before
value = cash * self.risk_degree / len(buy) if len(buy) > 0 else 0
# after
value = cash * self.get_risk_degree(trade_step) / len(buy) if len(buy) > 0 else 0

This was the only use of risk_degree in TopkDropoutStrategy, and it bypassed the getter. The change makes Topk consult get_risk_degree() for buy sizing, matching WeightStrategyBase (line 365) and its subclass EnhancedIndexingStrategy (line 482), which already call self.get_risk_degree(...). It is behavior-preserving for the default case, since BaseSignalStrategy.get_risk_degree() returns self.risk_degree by default.

Motivation and Context

Related issue: closes #2276.

get_risk_degree() is documented as the dynamic market-timing hook ("Dynamically risk_degree will result in Market timing.", signal_strategy.py:69). Because Topk read the raw attribute instead of the getter, a subclass overriding get_risk_degree() to return a time-varying multiplier (the documented way to implement market timing) had zero effect on a TopkDropout book — the override was never consulted for position sizing. Two strategies overriding the hook would therefore produce identical trades/returns. This fix makes the documented override mechanism actually work for TopkDropoutStrategy.

How Has This Been Tested?

  • Pass the test by running: pytest qlib/tests/test_all_pipeline.py under upper directory of qlib.
  • If you are adding a new feature, test on your own test scripts.

Added a regression test at tests/backtest/test_topk_risk_degree.py:

  • test_gate_to_cash_via_get_risk_degree — a TopkDropoutStrategy subclass that overrides get_risk_degree() to return 0.0 (gate to cash) now builds no stock position. Fails before this change (it bought stock using the raw 0.95 default), passes after.
  • test_base_strategy_does_buy — sanity check that the default strategy still buys.

Screenshots of Test Results (if appropriate):

  1. Pipeline test (tests/test_all_pipeline.py, default-risk-degree TopkDropoutStrategy backtest — confirms the default path is unchanged):
test_all_pipeline.py::TestAllFlow::test_0_train PASSED                   [ 33%]
test_all_pipeline.py::TestAllFlow::test_1_backtest PASSED                [ 66%]
test_all_pipeline.py::TestAllFlow::test_2_expmanager PASSED              [100%]
================== 3 passed, 3 warnings in 247.39s (0:04:07) ===================
  1. Your own tests (tests/backtest/test_topk_risk_degree.py):
backtest/test_topk_risk_degree.py::TopkRiskDegreeTest::test_base_strategy_does_buy PASSED [ 50%]
backtest/test_topk_risk_degree.py::TopkRiskDegreeTest::test_gate_to_cash_via_get_risk_degree PASSED [100%]
======================= 2 passed, 21 warnings in 27.09s ========================

Types of changes

  • Fix bugs

…izing

TopkDropoutStrategy.generate_trade_decision sized new buys with the raw
`self.risk_degree` attribute instead of `self.get_risk_degree(trade_step)`,
so a subclass overriding get_risk_degree() for market timing (the documented
mechanism: "Dynamically risk_degree will result in Market timing.") had zero
effect on the book. This brings it in line with WeightStrategyBase and
EnhancedIndexingStrategy, which already call the getter.

Add a regression test that gates gross exposure to cash via an overridden
get_risk_degree() and asserts the strategy builds no stock position.
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