fix financial/exponential_moving_average: seed uses window_size values, not window_size+1 - #15453
Open
aashish254 wants to merge 1 commit into
Open
aashish254 wants to merge 1 commit into
aashish254 wants to merge 1 commit into
Conversation
…s, not window_size+1 The initialization branch guarded with `if i <= window_size`, so it ran the SMA-seed branch for window_size+1 iterations before switching to the EMA recurrence. With window_size=1 (alpha=1) the documented recurrence should echo each input unchanged, but the seed branch averaged the first two prices into 15.0 instead of yielding 20.0, so exponential_moving_average(iter([10,20,30]), 1) returned [10, 15, 30]. Change the guard to `i < window_size`. The existing window_size=3 doctest is unchanged because alpha=0.5 makes the seed and EMA branches arithmetically identical at that specific index; add a window_size=1 doctest as the witness. Fixes TheAlgorithms#15449
This branch has not been deployed
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
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.
Describe your change
Checklist
Fixes #15449
What changes
One guard in
financial/exponential_moving_average.py:52:if i <= window_size:→if i < window_size:, plus awindow_size=1doctest as the witness.The seed branch ran for
window_size + 1iterations before the EMA recurrence started, so the first EMA update was skipped. Withwindow_size=1(alpha = 2 / (1 + 1) = 1) the documented recurrence must echo each input unchanged, but the seed averaged the first two prices into 15.0:The existing
window_size=3doctest is unchanged: atalpha=0.5the seed and EMA branches are arithmetically identical at index 3, so the sequence still equals(2, 3.5, 3.25, 5.725, 5.8625, 7.43125, 8.715625).The new doctest is the only test change; it is the regression witness for the one-line code fix, so code and doctest land together.
Testing
Mutation check — reverting only the guard back to
<=makes the new doctest fail with the exact reported values, confirming it is a real witness and not a tautology:Restoring the guard re-passes all doctests.