Skip to content

fix: SoftTopkStrategy sizes positions with get_risk_degree() - #2384

Open
xvruipeng-cyber wants to merge 2 commits into
microsoft:mainfrom
xvruipeng-cyber:fix/soft-topk-risk-degree
Open

xvruipeng-cyber wants to merge 2 commits into
microsoft:mainfrom
xvruipeng-cyber:fix/soft-topk-risk-degree

Conversation

@xvruipeng-cyber

Copy link
Copy Markdown

Summary

WeightStrategyBase already passes self.get_risk_degree(trade_step) into the order generator, but SoftTopkStrategy.generate_target_weight_position sized the book from the raw self.risk_degree attribute. A subclass that overrides the getter to change exposure therefore had no effect on target weights.

This uses the getter for both the per-name ideal weight and the rebalance budget. The default implementation still returns self.risk_degree, so existing books are unchanged. TopkDropoutStrategy has the same gap in #2276 and is already covered by #2277, so this PR does not touch it.

Test

pytest tests/backtest/test_soft_topk_strategy.py tests/backtest/test_soft_topk_strategy_cold_start.py: 4 passed. The new test sets risk_degree to 0.95 and overrides get_risk_degree() to return 0.5; cold-start weights follow 0.5.

Target weights read the risk_degree attribute, so a subclass override never affected the book while the order generator already used the getter.
@xvruipeng-cyber

xvruipeng-cyber commented Oct 22, 2026 via email

Copy link
Copy Markdown
Author

@arhancanli

Copy link
Copy Markdown

Read the diff next to signal_strategy.py. The getter signature is get_risk_degree(self, trade_step=None), and WeightStrategyBase.generate_trade_decision calls it as self.get_risk_degree(trade_step) with trade_step = self.trade_calendar.get_trade_step() (line 364 of my copy). The new line in SoftTopkStrategy.generate_target_weight_position calls self.get_risk_degree() with no argument, so an override that varies exposure by step gets None there and a real step index in the order generator. That is the kind of override this PR is meant to enable, and the two values would disagree.

self.trade_calendar.get_trade_step() is available inside the method (the base class uses it in the same call path), so passing it would keep both reads consistent. The new test overrides the getter with a constant 0.5, so it can't catch this; a variant whose return depends on trade_step (say 0.5 at step 0, 0.9 later) would. I read the code only and did not run the tests.

@xvruipeng-cyber

Copy link
Copy Markdown
Author

generate_target_weight_position now reads trade_calendar.get_trade_step() and passes that into get_risk_degree, the same step WeightStrategyBase.generate_trade_decision passes to the order generator. A direct call with no level_infra still uses None.

The new test overrides the getter to return 0.5 at step 0 and 0.9 afterwards. With the calendar on step 3, the recorded argument is 3 and the cold-start weights are 0.45 each, so a constant-0.5 override can no longer hide a missing step. tests/backtest/test_soft_topk_strategy.py: 3 passed.

@arhancanli

Arhan Canli (arhancanli) commented Oct 23, 2026 •

Copy link
Copy Markdown

Thanks. The cold-start check is a good one: with the override returning 0.5 at step 0 and 0.9 afterwards, the calendar on step 3 and the recorded argument 3, a constant override can no longer hide a missing step. I did not run the suite (code read only).

One more edge to pin: generate_target_weight_position now calls trade_calendar.get_trade_step(), so a strategy used without a level_infra goes through the None path. Is there a test for the case where level_infra is set but the calendar has not started (step before 0 or an uninitialised step)? That is the one place a getter taking the step could raise where the old no-argument call did not.

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.

2 participants