You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Fix repeated OHLC samples with a fixed random seed - #1420
An integer random_state currently gets passed to DataFrame.sample() on every shuffle. Pandas initializes a new RNG from that integer each time, so successive next() calls on random_ohlc_data() yield identical frames. This makes a seeded Monte Carlo/stress-test loop repeatedly evaluate the same generated data.
Initialize one RandomState per generator for integer seeds and reuse it for the shuffles. This keeps the sequence reproducible while allowing the state to advance. None and existing random-state objects continue to pass through to pandas. The docstring now explains the sequence semantics.
The regression tests cover variation between successive seeded samples and exact reproduction of a three-sample sequence across two generators with the same seed. The variation test fails on the unchanged implementation (first.equals(second) is True) and passes with the fix.
Validation on Windows / Python 3.11.9:
python -m backtesting.test: 83 tests, OK (1 Linux-only documentation test skipped).
flake8 backtesting setup.py: passed.
mypy --no-warn-unused-ignores backtesting: passed in a lint-only environment matching the dependency setup of the existing CI lint job.
git diff --check: passed.
For transparency, running mypy with the runtime dependencies installed (NumPy 2.4.6 / pandas 3.0.6) reports the same three existing typing errors in the unchanged upstream commit ca2e261 and the patched tree. Plain mypy backtesting in the lint-only environment also reports the same 15 existing unused-ignore findings in both trees; the existing CI command disables those warnings. This PR does not change those unrelated locations.
The diagnosis is right, and the reproducible-sequence test is the one I'd want. One side effect worth stating in the PR, because random_ohlc_data calls shuffle twice per sample (once for df, once for gaps):
With an integer seed the old code re-seeded both calls, so within one sample the bars and the gap series were permuted with the same permutation. With one shared RandomState the first call is unchanged and the second now draws a different permutation. I checked the primitive on pandas 2.3.3: DataFrame.sample(frac=1, random_state=3) called twice returns the same order both times, while two calls with one RandomState(3) return different orders and the first matches the old output. So for a given seed the first generated frame changes (the bars are identical, the gaps are not), and anyone who stored results from random_state=42 will not be able to reproduce them after this lands. I think the new behaviour is the better one, since the gaps are no longer tied to the bar order, but it deserves a line in the docstring or release note.
A test that would pin it down: with a fixed seed, assert that the deltas of the first sample differ from those computed with the same permutation for both shuffles (or just compare the first frame to a hard-coded value), so a later refactor can't silently go back to the shared permutation. Not run against the full suite; I read the diff and lib.py on current master.
This branch has not been deployed
No deployments
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
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.
An integer
random_statecurrently gets passed toDataFrame.sample()on every shuffle. Pandas initializes a new RNG from that integer each time, so successivenext()calls onrandom_ohlc_data()yield identical frames. This makes a seeded Monte Carlo/stress-test loop repeatedly evaluate the same generated data.Initialize one
RandomStateper generator for integer seeds and reuse it for the shuffles. This keeps the sequence reproducible while allowing the state to advance.Noneand existing random-state objects continue to pass through to pandas. The docstring now explains the sequence semantics.The regression tests cover variation between successive seeded samples and exact reproduction of a three-sample sequence across two generators with the same seed. The variation test fails on the unchanged implementation (
first.equals(second)isTrue) and passes with the fix.Validation on Windows / Python 3.11.9:
python -m backtesting.test: 83 tests, OK (1 Linux-only documentation test skipped).flake8 backtesting setup.py: passed.mypy --no-warn-unused-ignores backtesting: passed in a lint-only environment matching the dependency setup of the existing CI lint job.git diff --check: passed.For transparency, running mypy with the runtime dependencies installed (NumPy 2.4.6 / pandas 3.0.6) reports the same three existing typing errors in the unchanged upstream commit
ca2e261and the patched tree. Plainmypy backtestingin the lint-only environment also reports the same 15 existing unused-ignore findings in both trees; the existing CI command disables those warnings. This PR does not change those unrelated locations.