diff --git a/Indicators/PythonIndicator.cs b/Indicators/PythonIndicator.cs index ac63a90ec2b5..1fa74b0ffe38 100644 --- a/Indicators/PythonIndicator.cs +++ b/Indicators/PythonIndicator.cs @@ -26,9 +26,12 @@ namespace QuantConnect.Indicators public class PythonIndicator : IndicatorBase, IIndicatorWarmUpPeriodProvider { private static string _isReadyName = nameof(IsReady).ToSnakeCase(); + private static string _resetName = nameof(Reset).ToSnakeCase(); private PyObject _instance; private bool _isReady; private bool _pythonIsReadyProperty; + private PyObject _pythonResetMethod; + private bool _isResetting; private BasePythonWrapper _indicatorWrapper; /// @@ -94,6 +97,16 @@ public void SetIndicator(PyObject indicator) } } + using (Py.GIL()) + { + // Null when the attribute is absent, is a plain value rather than a method, or resolves + // to the CSharp implementation. Resolving here keeps Reset off the per-call GIL round trip. + // SetIndicator runs again on every GetIndicatorAsManagedObject call, so release first. + _pythonResetMethod?.Dispose(); + _pythonResetMethod = indicator.GetPythonMethodWithChecks(_resetName) as PyObject + ?? indicator.GetPythonMethodWithChecks(nameof(Reset)) as PyObject; + } + WarmUpPeriod = GetIndicatorWarmUpPeriod(); } @@ -128,6 +141,35 @@ public override bool IsReady /// public int WarmUpPeriod { get; protected set; } + /// + /// Resets this indicator to its initial state + /// + public override void Reset() + { + // For an inheriting class the wrapped instance is this same object, so a python reset() + // calling super().reset() arrives back here through the CLR binding. Invoke the python + // side once per reset and let the re-entrant call fall through to the base. + var reentrant = _isResetting; + try + { + if (_pythonResetMethod != null && !reentrant) + { + _isResetting = true; + using (Py.GIL()) + { + _pythonResetMethod.Invoke().Dispose(); + } + } + } + finally + { + // A python reset() that raises must still leave the CSharp side reset. + _isResetting = reentrant; + _isReady = false; + base.Reset(); + } + } + /// /// Computes the next value of this indicator from the given state /// diff --git a/Tests/Indicators/PythonIndicatorNoinheritanceTests.cs b/Tests/Indicators/PythonIndicatorNoinheritanceTests.cs index cafef8f5c64a..47586e6f0d65 100644 --- a/Tests/Indicators/PythonIndicatorNoinheritanceTests.cs +++ b/Tests/Indicators/PythonIndicatorNoinheritanceTests.cs @@ -62,6 +62,11 @@ def __init__(self, name, period): self.{(SnakeCase ? "value" : "Value")} = sum(self.queue) / count self.{(SnakeCase ? "is_ready" : "IsReady")} = count == self.queue.maxlen return self.{(SnakeCase ? "is_ready" : "IsReady")} + + def {(SnakeCase ? "reset" : "Reset")}(self): + self.queue.clear() + self.{(SnakeCase ? "value" : "Value")} = 0 + self.{(SnakeCase ? "is_ready" : "IsReady")} = False " ); var indicator = module.GetAttr("CustomSimpleMovingAverage") diff --git a/Tests/Indicators/PythonIndicatorNoinheritanceTestsLegacy.cs b/Tests/Indicators/PythonIndicatorNoinheritanceTestsLegacy.cs index b247d37f0c96..9331c09c50c4 100644 --- a/Tests/Indicators/PythonIndicatorNoinheritanceTestsLegacy.cs +++ b/Tests/Indicators/PythonIndicatorNoinheritanceTestsLegacy.cs @@ -61,6 +61,11 @@ def __init__(self, name, period): count = len(self.queue) self.{(SnakeCase ? "value" : "Value")} = sum(self.queue) / count self.{(SnakeCase ? "is_ready" : "IsReady")} = count == self.queue.maxlen + + def {(SnakeCase ? "reset" : "Reset")}(self): + self.queue.clear() + self.{(SnakeCase ? "value" : "Value")} = 0 + self.{(SnakeCase ? "is_ready" : "IsReady")} = False " ); var indicator = module.GetAttr("CustomSimpleMovingAverage") diff --git a/Tests/Indicators/PythonIndicatorResetTests.cs b/Tests/Indicators/PythonIndicatorResetTests.cs new file mode 100644 index 000000000000..3822b4fb9bca --- /dev/null +++ b/Tests/Indicators/PythonIndicatorResetTests.cs @@ -0,0 +1,168 @@ +/* + * QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. + * Lean Algorithmic Trading Engine v2.0. Copyright 2014 QuantConnect Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. +*/ + +using System; +using NUnit.Framework; +using Python.Runtime; +using QuantConnect.Indicators; + +namespace QuantConnect.Tests.Indicators +{ + [TestFixture] + public class PythonIndicatorResetSnakeCaseTests : PythonIndicatorResetTests + { + protected override bool SnakeCase => true; + } + + /// + /// Reset cases that build their own python class, so they do not use the indicator + /// the surrounding fixtures create. Kept separate from + /// for that reason: inheriting them there would run each case in four more fixtures + /// without changing anything it exercises. + /// + [TestFixture] + public class PythonIndicatorResetTests + { + protected virtual bool SnakeCase => false; + + private const int RecursionCap = 200; + + private static PythonIndicator CreateIndicatorFrom(string source, string className) + { + using (Py.GIL()) + { + var module = PyModule.FromString(Guid.NewGuid().ToString(), source); + var instance = module.GetAttr(className).Invoke(); + + return new PythonIndicator(instance); + } + } + + [Test] + public void ResetIsNotReenteredByASubclassCallingSuper() + { + using (Py.GIL()) + { + var module = PyModule.FromString( + Guid.NewGuid().ToString(), + $@" +from AlgorithmImports import * +from collections import deque + +class RecursiveReset(PythonIndicator): + depth = 0 + max_depth = 0 + + def __init__(self): + self.{(SnakeCase ? "name" : "Name")} = 'recursive' + self.{(SnakeCase ? "value" : "Value")} = 0 + self.queue = deque(maxlen=3) + + def {(SnakeCase ? "update" : "Update")}(self, input): + self.queue.appendleft(input.Value) + self.{(SnakeCase ? "value" : "Value")} = sum(self.queue) / len(self.queue) + return len(self.queue) == self.queue.maxlen + + def {(SnakeCase ? "reset" : "Reset")}(self): + cls = type(self) + cls.depth += 1 + cls.max_depth = max(cls.max_depth, cls.depth) + if cls.depth < {RecursionCap}: + super().{(SnakeCase ? "reset" : "Reset")}() + cls.depth -= 1 + self.queue.clear() +" + ); + + var pythonIndicator = module.GetAttr("RecursiveReset").Invoke(); + + // An inheriting class converts to its own CSharp part, so SetIndicator points the + // wrapper at this same object. This is what WrapPythonIndicator does on registration. + pythonIndicator.TryConvert(out PythonIndicator indicator); + Assert.IsNotNull(indicator); + indicator.SetIndicator(pythonIndicator); + + indicator.Update(new IndicatorDataPoint(new DateTime(2024, 1, 1), 100m)); + indicator.Reset(); + + // Zero would mean the python reset was never reached, the cap would mean it recursed. + var depth = module.GetAttr("RecursiveReset").GetAttr("max_depth").As(); + Assert.AreEqual(1, depth, $"python reset() ran {depth} deep"); + Assert.AreEqual(0, indicator.Samples); + Assert.AreEqual(0, pythonIndicator.GetAttr("queue").Length()); + } + } + + [Test] + public void ResetSkipsANonMethodResetAttribute() + { + using (Py.GIL()) + { + var indicator = CreateIndicatorFrom($@" +class PlainResetAttribute(): + def __init__(self): + self.{(SnakeCase ? "name" : "Name")} = 'plain' + self.{(SnakeCase ? "value" : "Value")} = 0 + self.{(SnakeCase ? "is_ready" : "IsReady")} = False + self.{(SnakeCase ? "reset" : "Reset")} = False + + def {(SnakeCase ? "update" : "Update")}(self, input): + self.{(SnakeCase ? "value" : "Value")} = input.Value + self.{(SnakeCase ? "is_ready" : "IsReady")} = True + return True +", "PlainResetAttribute"); + + indicator.Update(new IndicatorDataPoint(new DateTime(2024, 1, 1), 100m)); + + Assert.DoesNotThrow(() => indicator.Reset()); + Assert.AreEqual(0, indicator.Samples); + } + } + + [Test] + public void ResetClearsTheCSharpStateWhenPythonRaises() + { + using (Py.GIL()) + { + var indicator = CreateIndicatorFrom($@" +from AlgorithmImports import * +from collections import deque + +class RaisingReset(PythonIndicator): + def __init__(self): + self.{(SnakeCase ? "name" : "Name")} = 'raising' + self.{(SnakeCase ? "value" : "Value")} = 0 + self.queue = deque(maxlen=3) + + def {(SnakeCase ? "update" : "Update")}(self, input): + self.queue.appendleft(input.Value) + self.{(SnakeCase ? "value" : "Value")} = sum(self.queue) / len(self.queue) + return len(self.queue) == self.queue.maxlen + + def {(SnakeCase ? "reset" : "Reset")}(self): + raise ValueError('boom') +", "RaisingReset"); + + indicator.Update(new IndicatorDataPoint(new DateTime(2024, 1, 1), 100m)); + Assert.AreEqual(1, indicator.Samples); + + Assert.Throws(() => indicator.Reset()); + + Assert.AreEqual(0, indicator.Samples); + Assert.IsFalse(indicator.IsReady); + } + } + } +} diff --git a/Tests/Indicators/PythonIndicatorTests.cs b/Tests/Indicators/PythonIndicatorTests.cs index a2807ce13bd9..50cf4acb79e1 100644 --- a/Tests/Indicators/PythonIndicatorTests.cs +++ b/Tests/Indicators/PythonIndicatorTests.cs @@ -69,6 +69,10 @@ def __init__(self, name, period): count = len(self.queue) self.{(SnakeCase ? "value" : "Value")} = np.sum(self.queue) / count return count == self.queue.maxlen + + def {(SnakeCase ? "reset" : "Reset")}(self): + self.queue.clear() + self.{(SnakeCase ? "value" : "Value")} = 0 " ); var indicator = module.GetAttr("CustomSimpleMovingAverage") @@ -165,6 +169,30 @@ public void IsReadyAfterPeriodUpdates() Assert.IsTrue(sma.IsReady); } + [Test] + public void ResetClearsTheStateHeldInPython() + { + var indicator = CreateIndicator(); + var reference = new DateTime(2024, 1, 1); + + // Enough points to reach IsReady, otherwise asserting it is false after the reset + // passes on an indicator that was never ready. + for (var i = 0; i < 20; i++) + { + indicator.Update(new IndicatorDataPoint(reference.AddDays(i), 100m + i)); + } + Assert.IsTrue(indicator.IsReady); + + indicator.Reset(); + + Assert.AreEqual(0, indicator.Samples); + Assert.IsFalse(indicator.IsReady); + + indicator.Update(new IndicatorDataPoint(reference, 100m)); + + Assert.AreEqual(100m, indicator.Current.Value); + } + [Test] public override void ResetsProperly() {