Skip to content

Stop McClellanOscillator counting one shared bar twice - #9713

Open
mkzung wants to merge 1 commit into
QuantConnect:masterfrom
mkzung:bug-9711-mcclellan-warmup-period
Open

Stop McClellanOscillator counting one shared bar twice#9713
mkzung wants to merge 1 commit into
QuantConnect:masterfrom
mkzung:bug-9711-mcclellan-warmup-period

Conversation

@mkzung

@mkzung mkzung commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Description

McClellanOscillator.WarmUpPeriod adds EMASlow.WarmUpPeriod and ADDifference.WarmUpPeriod,
and those two overlap by one bar, so it reports 41 while the indicator is ready on 40.
McClellanSummationIndex delegates to it and reports the same. Subtracting the shared bar
fixes both.

Related Issue

Closes #9711

Motivation and Context

EMASlow is ADDifference.EMA(slowPeriod) and IndicatorExtensions.EMA passes
waitForFirstToReady = true, so EMASlow takes its first input on the bar ADDifference
becomes ready rather than the one after. Adding the periods counts that bar in both.

Nothing the indicator reports moves. At 40 bars the oscillator is 0 and the summation index
is 60, the same as at 41, and Samples is WarmUpPeriod * 3 either way, so the two value
assertions in those tests hold unchanged.

Requires Documentation Change

No.

How Has This Been Tested?

Both fixtures override WarmsUpProperly and assert IsTrue(IsReady) after WarmUpPeriod
bars without asserting it was false before, which is why an early ready passed. Each now
asserts readiness on every bar, the shape CommonIndicatorTests.WarmsUpProperly uses.

Four inherited tests also read WarmUpPeriod. TracksPreviousState, WorksWithLowValues
and IndicatorShouldHaveSymbolAfterUpdates use it only as a loop bound of 2 * period, and
WarmUpIndicatorProducesConsistentResults feeds exactly period bars per symbol and then
asserts IsReady, which this change makes tight rather than slack. Neither fixture overrides
any of the four.

My local NUnit host crashes in a Python.NET finalizer before any test runs, because
QuantConnect.pythonnet 2.0.65 wants Python 3.11 and this machine has 3.10, the same
limitation I noted on #9694. So this was measured from a console program linked against
QuantConnect.Indicators, feeding three symbols per bar as the fixtures do: ready on bar 40
against a reported 41, and reverting the change puts it back. The same program drives and
resets all 190 constructible indicators and finds no other disagreement between
WarmUpPeriod and IsReady.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My branch follows the naming convention bug-<issue#>-<description> or feature-<issue#>-<description>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

McClellanOscillator adds two warm-up periods that share a bar

1 participant