microsoft / microsoft/qlib

TopkDropoutStrategy ignores get_risk_degree() for position sizing

Open Beginner friendly
#2,276 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Python
Stars
48.7k
Forks
7.7k
PR merge metrics
No merged PRs in 30d

Description

🐛 Bug Description

get_risk_degree() is documented as the dynamic market-timing hook ("Dynamically risk_degree will result in Market timing.", qlib/contrib/strategy/signal_strategy.py:69). But TopkDropoutStrategy.generate_trade_decision sizes new buys with the raw attribute self.risk_degree, not the getter:

# qlib/contrib/strategy/signal_strategy.py:266
value = cash * self.risk_degree / len(buy) if len(buy) > 0 else 0

It is the only use of risk_degree in TopkDropoutStrategy, and it bypasses get_risk_degree(). So a subclass that overrides get_risk_degree() to return a time-varying multiplier (the documented way to implement market timing) has zero effect on a TopkDropout book — the override is never called. (By contrast, WeightStrategyBase at line 365 and its subclass EnhancedIndexingStrategy at line 482 do call self.get_risk_degree(...), so the inconsistency is within the same module.)

To Reproduce

  1. Subclass TopkDropoutStrategy and override get_risk_degree() to return, say, 0.0 on some dates (a regime gate to cash) and self.risk_degree otherwise.
  2. Backtest it vs. the un-subclassed TopkDropoutStrategy on the same signal.
  3. The two produce identical trades/returns — the overridden get_risk_degree() is never consulted for buy sizing.

Expected Behavior

TopkDropoutStrategy should size buys with self.get_risk_degree(trade_step) (as WeightStrategyBase does), so that overriding get_risk_degree() actually enables market timing as documented:

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

Environment

  • Qlib version: 0.9.7 (qlib/contrib/strategy/signal_strategy.py (TopkDropoutStrategy.generate_trade_decision, line ~266)
  • Python version: 3.12

Additional context

Discovered while building a BTC-trend regime overlay (RegimeGatedTopkStrategy(TopkDropoutStrategy)) that gates gross exposure via get_risk_degree(). The overlay had no effect on backtests until we worked around it by pushing the gated value onto self.risk_degree per step before delegating to super().generate_trade_decision(). A two-line fix in TopkDropoutStrategy (use the getter) would remove the need for the workaround.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read qlib/contrib/strategy/signal_strategy.py, especially TopkDropoutStrategy.generate_trade_decision around line 266 and the get_risk_degree() documentation near line 69. Compare its sizing path with WeightStrategyBase around line 365, then verify that an overridden getter affects buy sizing while the existing strategy behavior remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
fintech-quant
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.