Skip to content

Fix sliding window accumulator eviction order - #1640

Open
tenth-package0 wants to merge 2 commits into
google-deepmind:masterfrom
tenth-package0:codex/fix-sliding-window-average
Open

tenth-package0 wants to merge 2 commits into
google-deepmind:masterfrom
tenth-package0:codex/fix-sliding-window-average

Conversation

@tenth-package0

@tenth-package0 tenth-package0 commented Oct 9, 2026 •

Copy link
Copy Markdown

Summary

Fix the eviction order in SlidingWindowAccumulator. With a window size of 3, adding [1, 2, 3, 4] currently leaves [1, 2, 4] and returns a mean of 7 / 3, instead of retaining the latest three values and returning 3.

Keep the replacement index at zero while the buffer fills, then advance it only when replacing an entry. Use modular index advancement to make wraparound explicit. Reject non-positive capacities with a clear ValueError instead of failing later with an index error.

Add regression coverage for the first eviction, an empty window, invalid capacities, capacity one, partial windows, and repeated wraparound at several capacities. Register the test in the Python CMake test list.

Validation

  • python -m open_spiel.python.utils.stats_test: all 8 tests pass on Python 3.12.14; the original implementation fails the eviction regression cases.
  • ruff check open_spiel/python/utils/stats.py open_spiel/python/utils/stats_test.py: passed.
  • pyink --check open_spiel/python/utils/stats_test.py: passed.
  • git diff --check: passed.

The full C++/Python integration suite was not run locally.

@google-cla

google-cla Bot commented Oct 9, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

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.

1 participant

⌂ Proxy