Skip to content

Fix repeated OHLC samples with a fixed random seed - #1420

Open
machines-6657 wants to merge 1 commit into
kernc:masterfrom
machines-6657:codex/fix-seeded-ohlc-generator
Open

machines-6657 wants to merge 1 commit into
kernc:masterfrom
machines-6657:codex/fix-seeded-ohlc-generator

Conversation

@machines-6657

Copy link
Copy Markdown

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.

@arhancanli

Copy link
Copy Markdown

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
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.

2 participants