Skip to content

Commit 421d3c3

Browse files
authored
Fix RollingWindow element order corruption on resize (#9449)
* Fix RollingWindow resize * Fix MarketImpactSlippageModel * Reorder RollingWindow list in place
1 parent 6c2c4d5 commit 421d3c3

4 files changed

Lines changed: 69 additions & 7 deletions

File tree

Algorithm.CSharp/MarketImpactSlippageModelRegressionAlgorithm.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,11 @@ public override void OnOrderEvent(OrderEvent orderEvent)
9999
{"Drawdown", "4.200%"},
100100
{"Expectancy", "-1"},
101101
{"Start Equity", "10000000"},
102-
{"End Equity", "9649796.02"},
102+
{"End Equity", "9649801.20"},
103103
{"Net Profit", "-3.502%"},
104104
{"Sharpe Ratio", "-2.93"},
105105
{"Sortino Ratio", "-2.869"},
106-
{"Probabilistic Sharpe Ratio", "7.351%"},
106+
{"Probabilistic Sharpe Ratio", "7.352%"},
107107
{"Loss Rate", "100%"},
108108
{"Win Rate", "0%"},
109109
{"Profit-Loss Ratio", "0"},
@@ -119,7 +119,7 @@ public override void OnOrderEvent(OrderEvent orderEvent)
119119
{"Lowest Capacity Asset", "AAPL R735QTJ8XC9X"},
120120
{"Portfolio Turnover", "21.04%"},
121121
{"Drawdown Recovery", "0"},
122-
{"OrderListHash", "bee21851fd1ac425df8e01169d0db355"}
122+
{"OrderListHash", "fc0626f660981cb698f6a9a5d5d1389a"}
123123
};
124124
}
125125
}

Common/Indicators/RollingWindow.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,11 +368,23 @@ private void Resize(int size)
368368
{
369369
_listLock.EnterWriteLock();
370370

371-
_list.EnsureCapacity(size);
372371
if (size < _list.Count)
373372
{
373+
if (_tail != 0)
374+
{
375+
// The _list is out of order due to circular overwrites
376+
// Restore oldest to newest order and reset _tail before resizing
377+
var count = _list.Count;
378+
_list.Reverse(0, _tail);
379+
_list.Reverse(_tail, count - _tail);
380+
_list.Reverse(0, count);
381+
_tail = 0;
382+
}
383+
374384
_list.RemoveRange(0, _list.Count - size);
375385
}
386+
387+
_list.EnsureCapacity(size);
376388
_size = size;
377389
}
378390
finally

Common/Orders/Slippage/MarketImpactSlippageModel.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,13 +238,13 @@ public void OnDataConsolidated(object _, TradeBar bar)
238238
_prices.Add(bar.Close);
239239
_volumes.Add(bar.Volume);
240240

241-
if (_prices.Samples < 2)
241+
if (_prices.Count < 2)
242242
{
243243
return;
244244
}
245245

246-
var rocp = new double[_prices.Samples - 1];
247-
for (var i = 0; i < _prices.Samples - 1; i++)
246+
var rocp = new double[_prices.Count - 1];
247+
for (var i = 0; i < _prices.Count - 1; i++)
248248
{
249249
if (_prices[i + 1] == 0) continue;
250250

Tests/Indicators/RollingWindowTests.cs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,5 +602,55 @@ def rolling_window_with_custom_data_type():
602602
Assert.AreEqual(expectedValue, value);
603603
}
604604
}
605+
606+
[TestCase(2, 3, 5, new[] { 3, 2 })]
607+
[TestCase(5, 7, 3, new[] { 7, 6, 5 })]
608+
[TestCase(3, 6, 2, new[] { 6, 5 })]
609+
[TestCase(3, 6, 5, new[] { 6, 5, 4 })]
610+
public void ResizeFromOverwrittenWindowPreservesNewestElements(int initialSize, int totalAdds, int newSize, int[] expected)
611+
{
612+
var window = new RollingWindow<int>(initialSize);
613+
for (var i = 1; i <= totalAdds; i++)
614+
{
615+
window.Add(i);
616+
}
617+
618+
window.Size = newSize;
619+
620+
for (var i = 0; i < expected.Length; i++)
621+
{
622+
Assert.AreEqual(expected[i], window[i]);
623+
}
624+
}
625+
626+
[Test]
627+
public void MultipleResizesOnOverwrittenWindowPreserveElementOrder()
628+
{
629+
var window = new RollingWindow<int>(5);
630+
for (var i = 1; i <= 7; i++)
631+
{
632+
window.Add(i);
633+
}
634+
635+
window.Size = 3;
636+
Assert.AreEqual(7, window[0]);
637+
Assert.AreEqual(6, window[1]);
638+
Assert.AreEqual(5, window[2]);
639+
640+
window.Size = 10;
641+
Assert.AreEqual(7, window[0]);
642+
Assert.AreEqual(6, window[1]);
643+
Assert.AreEqual(5, window[2]);
644+
645+
for (var i = 8; i <= 17; i++)
646+
{
647+
window.Add(i);
648+
}
649+
650+
Assert.AreEqual(17, window[0]);
651+
Assert.AreEqual(16, window[1]);
652+
Assert.AreEqual(15, window[2]);
653+
Assert.AreEqual(8, window[9]);
654+
}
605655
}
606656
}

0 commit comments

Comments
 (0)