Clear the advance decline period watermark on reset - #9686
Merged
Martin-Molinero merged 1 commit intoAug 12, 2026
Merged
Conversation
AdvanceDeclineIndicator.Reset left _currentPeriodTime holding the last period it had seen. Enqueue only files a bar whose Time reaches that watermark, so after a reset every earlier bar was dropped silently and the indicator stayed unready for the whole replay. Clear the watermark along with the two period dictionaries. The McClellan test doubles seed the EMA values implied by the A/D history that precedes their CSV. That seeding ran once in the constructor, so the second half of ComparesAgainstExternalDataAfterReset compared unseeded EMAs against the file. Move it into their Reset override, which the constructor already calls.
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
AdvanceDeclineIndicator.Reset()clears both period dictionaries but leaves_currentPeriodTimeholding the last period it processed.Enqueueonly files a bar whoseTimereaches that watermark, so after a reset every earlier bar is dropped and the indicator never becomes ready again. Clearing the watermark alongside the dictionaries is the whole fix.The test doubles in
McClellanOscillatorTestsandMcClellanSummationIndexTestsseed the EMA values implied by the A/D history preceding their CSV. That seeding ran once, in the constructor, so it did not survive a reset. It moves into theirResetoverride, which the constructor already calls. Without this the two fixtures fail once the indicator starts producing values after a reset, which is the point of the change.Related Issue
Fixes #9683.
Motivation and Context
Reset is meant to return an indicator to its constructed state. Here it left one field set, and the field gates whether any further data is accepted at all, so the indicator went quiet rather than wrong. 162 of 180 updates differ from a fresh instance over 60 daily periods across 3 symbols, and 171 of 180 differ in
IsReady.AdvanceDeclineDifference,AdvanceDeclineRatio,AdvanceDeclineVolumeRatio,McClellanOscillatorandMcClellanSummationIndexall build on this indicator.Requires Documentation Change
No.
How Has This Been Tested?
ResetsProperlyAndReplaysTheSameValuesinAdvanceDeclineDifferenceTestsfeeds the indicator, resets it, replays the same bars, and compares each value andIsReadyagainst a fresh instance. It fails onmasterand passes with the fix. Being in the shared A/D fixture, it is inherited by the ratio and volume-ratio fixtures too.QuantConnect.Tests.Indicatorson a clean checkout of this branch: 2768 passed, 0 failed, 5 skipped, on .NET 9 under Linux.Worth flagging separately:
TestHelper.AssertIndicatorHasExternalDataAfterResetskips its assertion while the indicator is not ready, so onmasterthe second pass ofComparesAgainstExternalDataAfterResetasserts 0 of 756 rows for all three A/D indicators and passes vacuously. With this fix it asserts 753 of 756. That helper is inherited by roughly 200 fixtures, so tightening it is a separate discussion rather than part of this PR.Types of changes
Checklist:
bug-<issue#>-<description>