Skip to content

Conversation

@feedseawave
Copy link

@feedseawave feedseawave commented Dec 30, 2025

Description

This PR refactors the SoftTopkStrategy to address the non-deterministic weight allocation issue during portfolio rebalancing.

Motivation and Context

  • Fix Portfolio Overflow Logic: Addressed the issue where final_stock_weight was incorrectly handled when the number of held stocks exceeded topk (common when trade_impact_limit prevents immediate liquidation of old positions).
  • Code Cleanup: Improved code readability by aligning comments and standardizing variable naming conventions
  • Tests: Added coverage for rebalancing and cold-start scenarios.

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.

Screenshots of Test Results (if appropriate):

  1. Pipeline test:
  2. Your own tests:
test1 test2

Types of changes

  • Fix bugs
  • Add new feature
  • Update documentation

@feedseawave
Copy link
Author

@microsoft-github-policy-service agree

@feedseawave
Copy link
Author

Hi @SunsetWolf, sorry for the message. I noticed you’ve been active on recent fixes. Just a friendly ping on this PR. I’ve checked the CI and all tests have passed. Is there any additional information or changes needed from my side to move this forward? Thanks for your time!"

order_generator_cls_or_obj=OrderGenWInteract,
max_sold_weight=1.0,
trade_impact_limit=None,
priority="IMPACT_FIRST",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't have to define an extra parameter.
Setting max_sold_weight=1.0 indicates COMPLIANCE_FIRST

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. I removed the unnecessary parameter and now only use trade_impact_limit/max_sold_weight to control the cap. Logic and tests have been updated accordingly.

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