Skip to content

Commit d3a2c7f

Browse files
Martin-MolineroMartin-Molinero
committed
Address reviews
- Moving InsightCollection into base `PortfolioConstructionModel` - Will call `InsightCollection.GetNextExpiryTime()` on each check, and for performance `InsightCollection` will keep track of next insight expiry time - Removing need for PCM base classes having to call `RefreshRebalance` - Some refactor clean up at base PortfolioConstructionModel.IsRebalanceDue()
1 parent e6d07d6 commit d3a2c7f

10 files changed

Lines changed: 86 additions & 118 deletions

‎Algorithm.CSharp/PortfolioRebalanceOnInsightChangesRegressionAlgorithm.cs‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -87,31 +87,31 @@ public override void OnOrderEvent(OrderEvent orderEvent)
8787
/// </summary>
8888
public Dictionary<string, string> ExpectedStatistics => new Dictionary<string, string>
8989
{
90-
{"Total Trades", "82"},
91-
{"Average Win", "0.16%"},
92-
{"Average Loss", "-0.03%"},
93-
{"Compounding Annual Return", "10.320%"},
90+
{"Total Trades", "84"},
91+
{"Average Win", "0.14%"},
92+
{"Average Loss", "-0.04%"},
93+
{"Compounding Annual Return", "9.798%"},
9494
{"Drawdown", "18.100%"},
95-
{"Expectancy", "4.025"},
96-
{"Net Profit", "21.705%"},
97-
{"Sharpe Ratio", "0.616"},
98-
{"Probabilistic Sharpe Ratio", "27.555%"},
99-
{"Loss Rate", "25%"},
100-
{"Win Rate", "75%"},
101-
{"Profit-Loss Ratio", "5.70"},
102-
{"Alpha", "0.091"},
103-
{"Beta", "0.016"},
104-
{"Annual Standard Deviation", "0.15"},
105-
{"Annual Variance", "0.022"},
106-
{"Information Ratio", "0.162"},
107-
{"Tracking Error", "0.197"},
108-
{"Treynor Ratio", "5.64"},
109-
{"Total Fees", "$82.78"},
95+
{"Expectancy", "2.258"},
96+
{"Net Profit", "20.557%"},
97+
{"Sharpe Ratio", "0.575"},
98+
{"Probabilistic Sharpe Ratio", "25.475%"},
99+
{"Loss Rate", "22%"},
100+
{"Win Rate", "78%"},
101+
{"Profit-Loss Ratio", "3.16"},
102+
{"Alpha", "0.088"},
103+
{"Beta", "0.012"},
104+
{"Annual Standard Deviation", "0.155"},
105+
{"Annual Variance", "0.024"},
106+
{"Information Ratio", "0.143"},
107+
{"Tracking Error", "0.201"},
108+
{"Treynor Ratio", "7.403"},
109+
{"Total Fees", "$84.80"},
110110
{"Fitness Score", "0.001"},
111111
{"Kelly Criterion Estimate", "0"},
112112
{"Kelly Criterion Probability Value", "1"},
113-
{"Sortino Ratio", "0.859"},
114-
{"Return Over Maximum Drawdown", "0.57"},
113+
{"Sortino Ratio", "0.812"},
114+
{"Return Over Maximum Drawdown", "0.541"},
115115
{"Portfolio Turnover", "0.002"},
116116
{"Total Insights Generated", "2028"},
117117
{"Total Insights Closed", "2024"},

‎Algorithm.Framework/Portfolio/BlackLittermanOptimizationPortfolioConstructionModel.cs‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,8 @@ public class BlackLittermanOptimizationPortfolioConstructionModel : PortfolioCon
4343
private readonly double _tau;
4444
private readonly IPortfolioOptimizer _optimizer;
4545

46-
private DateTime? _nextExpiryTime;
47-
4846
private List<Symbol> _removedSymbols;
4947
private readonly Dictionary<Symbol, ReturnsSymbolData> _symbolDataDict;
50-
private readonly InsightCollection _insightCollection = new InsightCollection();
5148

5249
/// <summary>
5350
/// Initialize the model
@@ -139,7 +136,7 @@ public override IEnumerable<IPortfolioTarget> CreateTargets(QCAlgorithm algorith
139136
if (insights.Length > 0)
140137
{
141138
insights = FilterInvalidInsightMagnitude(algorithm, insights);
142-
_insightCollection.AddRange(insights);
139+
InsightCollection.AddRange(insights);
143140
}
144141

145142
if (!IsRebalanceDue(insights, algorithm.UtcTime))
@@ -158,7 +155,7 @@ public override IEnumerable<IPortfolioTarget> CreateTargets(QCAlgorithm algorith
158155
}
159156

160157
// Get insight that haven't expired of each symbol that is still in the universe
161-
var activeInsights = _insightCollection.GetActiveInsights(algorithm.UtcTime);
158+
var activeInsights = InsightCollection.GetActiveInsights(algorithm.UtcTime);
162159

163160
// Get the last generated active insight for each symbol
164161
var lastActiveInsights = (from insight in activeInsights
@@ -210,18 +207,15 @@ select g.OrderBy(x => x.GeneratedTimeUtc).Last())
210207
}
211208
}
212209
// Get expired insights and create flatten targets for each symbol
213-
var expiredInsights = _insightCollection.RemoveExpiredInsights(algorithm.UtcTime);
210+
var expiredInsights = InsightCollection.RemoveExpiredInsights(algorithm.UtcTime);
214211

215212
var expiredTargets = from insight in expiredInsights
216213
group insight.Symbol by insight.Symbol into g
217-
where !_insightCollection.HasActiveInsights(g.Key, algorithm.UtcTime)
214+
where !InsightCollection.HasActiveInsights(g.Key, algorithm.UtcTime)
218215
select new PortfolioTarget(g.Key, 0);
219216

220217
targets.AddRange(expiredTargets);
221218

222-
_nextExpiryTime = _insightCollection.GetNextExpiryTime();
223-
RefreshRebalance(algorithm.UtcTime, _nextExpiryTime);
224-
225219
return targets;
226220
}
227221

@@ -235,7 +229,7 @@ public override void OnSecuritiesChanged(QCAlgorithm algorithm, SecurityChanges
235229
base.OnSecuritiesChanged(algorithm, changes);
236230
// Get removed symbol and invalidate them in the insight collection
237231
_removedSymbols = changes.RemovedSecurities.Select(x => x.Symbol).ToList();
238-
_insightCollection.Clear(_removedSymbols.ToArray());
232+
InsightCollection.Clear(_removedSymbols.ToArray());
239233

240234
foreach (var symbol in _removedSymbols)
241235
{

‎Algorithm.Framework/Portfolio/BlackLittermanOptimizationPortfolioConstructionModel.py‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,6 @@ def __init__(self,
7373
self.removedSymbols = []
7474
self.symbolDataBySymbol = {}
7575

76-
self.insightCollection = InsightCollection()
77-
7876
# If the argument is an instance of Resolution or Timedelta
7977
# Redefine rebalancingFunc
8078
rebalancingFunc = rebalancingParam
@@ -98,7 +96,7 @@ def CreateTargets(self, algorithm, insights):
9896

9997
# Always add new insights
10098
insights = PortfolioConstructionModel.FilterInvalidInsightMagnitude(algorithm, insights)
101-
self.insightCollection.AddRange(insights)
99+
self.InsightCollection.AddRange(insights)
102100

103101
if not self.IsRebalanceDue(insights, algorithm.UtcTime):
104102
return targets
@@ -110,7 +108,7 @@ def CreateTargets(self, algorithm, insights):
110108
self.removedSymbols = None
111109

112110
# Get insight that haven't expired of each symbol that is still in the universe
113-
activeInsights = self.insightCollection.GetActiveInsights(algorithm.UtcTime)
111+
activeInsights = self.InsightCollection.GetActiveInsights(algorithm.UtcTime)
114112

115113
# Get the last generated active insight for each symbol
116114
lastActiveInsights = []
@@ -152,19 +150,16 @@ def CreateTargets(self, algorithm, insights):
152150
targets.append(target)
153151

154152
# Get expired insights and create flatten targets for each symbol
155-
expiredInsights = self.insightCollection.RemoveExpiredInsights(algorithm.UtcTime)
153+
expiredInsights = self.InsightCollection.RemoveExpiredInsights(algorithm.UtcTime)
156154

157155
expiredTargets = []
158156
for symbol, f in groupby(expiredInsights, lambda x: x.Symbol):
159-
if not self.insightCollection.HasActiveInsights(symbol, algorithm.UtcTime):
157+
if not self.InsightCollection.HasActiveInsights(symbol, algorithm.UtcTime):
160158
expiredTargets.append(PortfolioTarget(symbol, 0))
161159
continue
162160

163161
targets.extend(expiredTargets)
164162

165-
nextExpiryTime = self.insightCollection.GetNextExpiryTime()
166-
self.RefreshRebalance(algorithm.UtcTime, nextExpiryTime)
167-
168163
return targets
169164

170165

@@ -177,7 +172,7 @@ def OnSecuritiesChanged(self, algorithm, changes):
177172
# Get removed symbol and invalidate them in the insight collection
178173
super().OnSecuritiesChanged(algorithm, changes)
179174
self.removedSymbols = [x.Symbol for x in changes.RemovedSecurities]
180-
self.insightCollection.Clear(self.removedSymbols)
175+
self.InsightCollection.Clear(self.removedSymbols)
181176

182177
for symbol in self.removedSymbols:
183178
symbolData = self.symbolDataBySymbol.pop(symbol, None)

‎Algorithm.Framework/Portfolio/EqualWeightingPortfolioConstructionModel.cs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ namespace QuantConnect.Algorithm.Framework.Portfolio
3030
public class EqualWeightingPortfolioConstructionModel : PortfolioConstructionModel
3131
{
3232
private List<Symbol> _removedSymbols;
33-
private readonly InsightCollection _insightCollection = new InsightCollection();
3433

3534
/// <summary>
3635
/// Initialize a new instance of <see cref="EqualWeightingPortfolioConstructionModel"/>
@@ -101,7 +100,7 @@ public override IEnumerable<IPortfolioTarget> CreateTargets(QCAlgorithm algorith
101100
if (insights.Length > 0)
102101
{
103102
// Validate we should create a target for this insight
104-
_insightCollection.AddRange(insights.Where(ShouldCreateTargetForInsight));
103+
InsightCollection.AddRange(insights.Where(ShouldCreateTargetForInsight));
105104
}
106105

107106
if (!IsRebalanceDue(insights, algorithm.UtcTime))
@@ -120,7 +119,7 @@ public override IEnumerable<IPortfolioTarget> CreateTargets(QCAlgorithm algorith
120119
}
121120

122121
// Get insight that haven't expired of each symbol that is still in the universe
123-
var activeInsights = _insightCollection.GetActiveInsights(algorithm.UtcTime);
122+
var activeInsights = InsightCollection.GetActiveInsights(algorithm.UtcTime);
124123

125124
// Get the last generated active insight for each symbol
126125
var lastActiveInsights = (from insight in activeInsights
@@ -146,18 +145,15 @@ group insight by insight.Symbol into g
146145
}
147146

148147
// Get expired insights and create flatten targets for each symbol
149-
var expiredInsights = _insightCollection.RemoveExpiredInsights(algorithm.UtcTime);
148+
var expiredInsights = InsightCollection.RemoveExpiredInsights(algorithm.UtcTime);
150149

151150
var expiredTargets = from insight in expiredInsights
152151
group insight.Symbol by insight.Symbol into g
153-
where !_insightCollection.HasActiveInsights(g.Key, algorithm.UtcTime) && !errorSymbols.Contains(g.Key)
152+
where !InsightCollection.HasActiveInsights(g.Key, algorithm.UtcTime) && !errorSymbols.Contains(g.Key)
154153
select new PortfolioTarget(g.Key, 0);
155154

156155
targets.AddRange(expiredTargets);
157156

158-
var nextExpiryTimeUtc = _insightCollection.GetNextExpiryTime();
159-
RefreshRebalance(algorithm.UtcTime, nextExpiryTimeUtc);
160-
161157
return targets;
162158
}
163159

@@ -171,7 +167,7 @@ public override void OnSecuritiesChanged(QCAlgorithm algorithm, SecurityChanges
171167
base.OnSecuritiesChanged(algorithm, changes);
172168
// Get removed symbol and invalidate them in the insight collection
173169
_removedSymbols = changes.RemovedSecurities.Select(x => x.Symbol).ToList();
174-
_insightCollection.Clear(_removedSymbols.ToArray());
170+
InsightCollection.Clear(_removedSymbols.ToArray());
175171
}
176172
}
177173
}

‎Algorithm.Framework/Portfolio/EqualWeightingPortfolioConstructionModel.py‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ def __init__(self, rebalancingParam = Resolution.Daily):
3232
Args:
3333
rebalancingParam: Rebalancing parameter. If it is a timedelta or Resolution, it will be converted into a function.
3434
The function returns the next expected rebalance time for a given algorithm UTC DateTime'''
35-
self.insightCollection = InsightCollection()
3635
self.removedSymbols = []
3736

3837
# If the argument is an instance of Resolution or Timedelta
@@ -77,7 +76,7 @@ def CreateTargets(self, algorithm, insights):
7776
# Always add new insights
7877
for insight in insights:
7978
if self.ShouldCreateTargetForInsight(insight):
80-
self.insightCollection.Add(insight)
79+
self.InsightCollection.Add(insight)
8180

8281
if not self.IsRebalanceDue(insights, algorithm.UtcTime):
8382
return targets
@@ -89,7 +88,7 @@ def CreateTargets(self, algorithm, insights):
8988
self.removedSymbols = None
9089

9190
# Get insight that haven't expired of each symbol that is still in the universe
92-
activeInsights = self.insightCollection.GetActiveInsights(algorithm.UtcTime)
91+
activeInsights = self.InsightCollection.GetActiveInsights(algorithm.UtcTime)
9392

9493
# Get the last generated active insight for each symbol
9594
lastActiveInsights = []
@@ -108,19 +107,16 @@ def CreateTargets(self, algorithm, insights):
108107
errorSymbols[insight.Symbol] = insight.Symbol
109108

110109
# Get expired insights and create flatten targets for each symbol
111-
expiredInsights = self.insightCollection.RemoveExpiredInsights(algorithm.UtcTime)
110+
expiredInsights = self.InsightCollection.RemoveExpiredInsights(algorithm.UtcTime)
112111

113112
expiredTargets = []
114113
for symbol, f in groupby(expiredInsights, lambda x: x.Symbol):
115-
if not self.insightCollection.HasActiveInsights(symbol, algorithm.UtcTime) and not symbol in errorSymbols:
114+
if not self.InsightCollection.HasActiveInsights(symbol, algorithm.UtcTime) and not symbol in errorSymbols:
116115
expiredTargets.append(PortfolioTarget(symbol, 0))
117116
continue
118117

119118
targets.extend(expiredTargets)
120119

121-
nextExpiryTime = self.insightCollection.GetNextExpiryTime()
122-
self.RefreshRebalance(algorithm.UtcTime, nextExpiryTime)
123-
124120
return targets
125121

126122
def OnSecuritiesChanged(self, algorithm, changes):
@@ -132,4 +128,4 @@ def OnSecuritiesChanged(self, algorithm, changes):
132128
# Get removed symbol and invalidate them in the insight collection
133129
super().OnSecuritiesChanged(algorithm, changes)
134130
self.removedSymbols = [x.Symbol for x in changes.RemovedSecurities]
135-
self.insightCollection.Clear(self.removedSymbols)
131+
self.InsightCollection.Clear(self.removedSymbols)

‎Algorithm.Framework/Portfolio/MeanVarianceOptimizationPortfolioConstructionModel.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,6 @@ public override IEnumerable<IPortfolioTarget> CreateTargets(QCAlgorithm algorith
183183
sidx++;
184184
}
185185
}
186-
RefreshRebalance(algorithm.UtcTime);
187186

188187
return targets;
189188
}

‎Algorithm.Framework/Portfolio/MeanVarianceOptimizationPortfolioConstructionModel.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ def CreateTargets(self, algorithm, insights):
108108
target = PortfolioTarget.Percent(algorithm, insight.Symbol, weight)
109109
if target is not None:
110110
targets.append(target)
111-
self.RefreshRebalance(algorithm.UtcTime)
112111

113112
return targets
114113

‎Algorithm/Portfolio/PortfolioConstructionModel.cs‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ public class PortfolioConstructionModel : IPortfolioConstructionModel
3030
{
3131
private Func<DateTime, DateTime?> _rebalancingFunc;
3232
private DateTime? _rebalancingTime;
33-
private DateTime? _nextExpiryTime;
3433
private bool _securityChanges;
3534

3635
/// <summary>
@@ -43,13 +42,21 @@ public class PortfolioConstructionModel : IPortfolioConstructionModel
4342
/// </summary>
4443
public static bool RebalanceOnInsightChanges { get; set; } = true;
4544

45+
/// <summary>
46+
/// Provides a collection for managing insights
47+
/// </summary>
48+
/// <remarks>Derived classes should use this collection if they want insight
49+
/// expiration to trigger a rebalance</remarks>
50+
protected InsightCollection InsightCollection { get; }
51+
4652
/// <summary>
4753
/// Initialize a new instance of <see cref="PortfolioConstructionModel"/>
4854
/// </summary>
4955
/// <param name="rebalancingFunc">For a given algorithm UTC DateTime returns the next expected rebalance time</param>
5056
public PortfolioConstructionModel(Func<DateTime, DateTime?> rebalancingFunc)
5157
{
5258
_rebalancingFunc = rebalancingFunc;
59+
InsightCollection = new InsightCollection();
5360
}
5461

5562
/// <summary>
@@ -93,49 +100,50 @@ protected void SetRebalancingFunc(PyObject rebalancingFunc)
93100
}
94101

95102
/// <summary>
96-
/// Determines if the portfolio should be rebalanced base on the provided rebalancing func
97-
/// and if any security change have been taken place.
103+
/// Determines if the portfolio should be rebalanced base on the provided rebalancing func,
104+
/// if any security change have been taken place or if an insight has expired or a new insight arrived
98105
/// If the rebalancing function has not been provided will return true.
99106
/// </summary>
100107
/// <param name="insights">The insights to create portfolio targets from</param>
101108
/// <param name="algorithmUtc">The current algorithm UTC time</param>
102109
/// <returns>True if should rebalance</returns>
103110
protected virtual bool IsRebalanceDue(Insight[] insights, DateTime algorithmUtc)
104111
{
105-
if (RebalanceOnInsightChanges
106-
&& (insights.Length != 0
107-
|| _nextExpiryTime != null && _nextExpiryTime < algorithmUtc))
108-
{
109-
return true;
110-
}
111-
112+
// if there is no rebalance func set, just return true but refresh state
113+
// just in case the rebalance func is going to be set.
112114
if (_rebalancingFunc == null)
113115
{
116+
RefreshRebalance(algorithmUtc);
114117
return true;
115118
}
116119

120+
// we always get the next expiry time
121+
// we don't know if a new insight was added or removed
122+
var nextInsightExpiryTime = InsightCollection.GetNextExpiryTime();
123+
117124
if (_rebalancingTime == null)
118125
{
119-
RefreshRebalance(algorithmUtc, _nextExpiryTime);
126+
_rebalancingTime = _rebalancingFunc(algorithmUtc);
120127
}
121128

122-
if ((_rebalancingTime != null && _rebalancingTime < algorithmUtc)
123-
|| (RebalanceOnSecurityChanges && _securityChanges))
129+
if (_rebalancingTime != null && _rebalancingTime < algorithmUtc
130+
|| RebalanceOnSecurityChanges && _securityChanges
131+
|| RebalanceOnInsightChanges
132+
&& (insights.Length != 0
133+
|| nextInsightExpiryTime != null && nextInsightExpiryTime < algorithmUtc))
124134
{
125-
RefreshRebalance(algorithmUtc, _nextExpiryTime);
126-
_securityChanges = false;
135+
RefreshRebalance(algorithmUtc);
127136
return true;
128137
}
129138

130139
return false;
131140
}
132141

133142
/// <summary>
134-
/// Refresh the next rebalance time
143+
/// Refresh the next rebalance time and clears the security changes flag
135144
/// </summary>
136-
protected void RefreshRebalance(DateTime algorithmUtc, DateTime? nextExpiration = null)
145+
private void RefreshRebalance(DateTime algorithmUtc)
137146
{
138-
_nextExpiryTime = nextExpiration;
139147
if (_rebalancingFunc != null)
140148
{
141149
_rebalancingTime = _rebalancingFunc(algorithmUtc);

0 commit comments

Comments
 (0)