Lean icon indicating copy to clipboard operation
Lean copied to clipboard

Suggestions for Improvements to `PortfolioConstructionModel`

Open ArthurAsenheimer opened this issue 10 months ago • 1 comments

Expected Behavior

  1. The method GetTargetInsights should correctly handle insights emitted simultaneously from multiple Alpha Models.
  2. The method DetermineTargetPercent should provide target portfolio weights per symbol instead of per insight.

Actual Behavior

  1. Currently, GetTargetInsights only supports a single AlphaModel. If two or more Alpha Models emit insights simultaneously, only the insights from the most recently added AlphaModel are considered, ignoring the other AlphaModel entirely. See also this backtest.
  2. DetermineTargetPercent returns a Dictionary<Insight, double>. Assigning weights based on insights is uncommon and impractical since insights themselves cannot be traded. Additionally, having multiple active insights for the same symbol results in multiple PortfolioTarget objects for that symbol at the same time, causing unnecessay trades in the ExecutionModel.

Potential Solution

  1. Adjust GetTargetInsights to group insights by both Symbol and AlphaModel/SourceModel, providing condideration of insights from all Alpha Models.
  2. Since method overloading by return type isn't possible, consider either:
    • Introducing an overload method such as: Dictionary<Symbol, double> DetermineTargetPercent(List<Insight> activeInsights, bool groupBySymbol = false)
    • Or creating a separate method explicitly named: DetermineTargetPercentBySymbol()

Open for additional suggestions or alternative ideas. 😊

Checklist

  • [x] I have completely filled out this template
  • [x] I have confirmed that this issue exists on the current master branch
  • [x] I have confirmed that this is not a duplicate issue by searching issues

ArthurAsenheimer avatar Mar 14 '25 10:03 ArthurAsenheimer

  1. The InsightManager is supposed to be cleaned up regularly to only keep the last active insight for each (AlphaModel, Symbol) pair. I don't see a use case where people need older insight objects that were already overriden by a newer one. For backwards compatibility we could make it optional with a setting like settings.auto_cleanup_insights = False (default). This will also help a lot in terms of performance as it will keep the number of insights as small as possible.

ArthurAsenheimer avatar Sep 08 '25 14:09 ArthurAsenheimer