test_dpo_learns_preferences is flaky

[DenisDrobyshev]

Corrected. The first version of this issue claimed seed= never reaches torch and that nothing in the package calls manual_seed. That is wrong, and the evidence I based it on was bad — I used GitHub's code search API, which returns zero matches for this repository because it is not indexed. It also returns zero for torch in praxis, which is how I caught it. Seeding here is fine; the flake is real but has another cause. The original text is below the line.

tests/test_rlhf.py::test_dpo_learns_preferences is flaky.

>       assert metrics["accuracy"] > 0.8
E       assert 0.755625 > 0.8
tests/test_rlhf.py:59: AssertionError

The flake, established

• It failed on a branch that adds one file, CITATION.cff, and changes nothing else.
• A workflow_dispatch run on untouched main minutes later passed every leg, including 3.12.
• Re-running the same failed job on the same commit, with nothing changed, passed: lint + fast tests (py3.12) -> success.

Identical commit, identical interpreter, identical resolved dependencies, opposite results.

What is already correct, and rules out the obvious causes

tests/conftest.py seeds every RNG before each test through an autouse fixture, and its docstring already names the exact hazard:

Network initialization uses the global PyTorch RNG, so without this a test's outcome could depend on which tests ran before it.

set_seed covers random, numpy, PYTHONHASHSEED, torch.manual_seed and torch.cuda.manual_seed_all. And ci.yml and verify.yml both pin OMP_NUM_THREADS: "1", so thread-count variation in float accumulation is already off the table.

So this is not a seeding bug and not a threading bug. Both were handled before I looked.

What is left

Two candidates, neither verified:

1. Runner hardware. ubuntu-latest is a heterogeneous fleet. Different vector instruction sets take different reduction paths, the last bits differ, and 600 training iterations amplify that. This fits the evidence exactly — identical software, different machine.
2. A genuine bimodality in the run. A five-point gap in final accuracy is large for pure float jitter, which would point at training landing in a different basin depending on a tiny initial difference rather than at rounding.

set_seed(..., deterministic=True) is not currently used by the tests, and would close the first of those if it is the cause.

The part I would still change

Guessing between those two is the wrong next move. The useful one is to measure: run this test twenty or fifty times in CI and look at the distribution of metrics["accuracy"]. That answers whether 0.7556 is a tail or a second mode, and it turns the threshold into something chosen rather than assumed.

Whatever it shows, > 0.8 is a hard threshold on a trained quantity with no stated tolerance. The two alternatives that survive a machine change:

• assert DPO ranks held-out preferences better than an untrained model by a margin — a claim about the method rather than about one number;
• assert a lower bound over several seeds rather than a point from one.

The second is module 1 of lemma applied to this repository's own suite.

Blocking

Nothing now — decisionrl#14 went green on a re-run and is merged. But any run can flake until this is characterised.

[DenisDrobyshev]

The claim in the first version of this issue — that seed= never reaches torch — was wrong, and worth saying plainly since it named a bug that does not exist.

I based it on GitHub's code search API returning zero matches for manual_seed in this repository. It also returns zero for torch in praxis, which cannot be true either: the API simply is not indexing these repositories. A grep of the actual source finds set_seed in utils/seeding.py covering random, numpy, PYTHONHASHSEED, torch.manual_seed and torch.cuda.manual_seed_all, called from an autouse fixture in tests/conftest.py before every test — with a docstring that already names the global-RNG hazard I claimed was unhandled.

OMP_NUM_THREADS: "1" is likewise already pinned in ci.yml and verify.yml.

The flake itself is real and reproduced three ways, so the issue stays open — but with the seeding claim removed and the remaining candidates marked as unverified rather than asserted. Body updated.

[DenisDrobyshev]

A second instance, and a more dramatic one.

tests/test_recurrent.py::test_recurrent_ppo_learns_cartpole failed on a branch whose entire diff is .github/dependabot.yml:

>       assert mean > 60, f"RecurrentPPO failed to learn CartPole (mean_return={mean:.1f})"
E       AssertionError: RecurrentPPO failed to learn CartPole (mean_return=19.6)
E       assert 19.65 > 60

Re-running that job on the same commit, with nothing changed, passed.

Why this one is worse than the DPO case

The DPO failure was marginal — 0.7556 against a 0.8 threshold, the kind of gap that could be read as a tail. This one is not marginal. A random policy on CartPole scores around 20, and 19.65 is exactly that. The run did not underperform; it learned nothing at all, then learned normally on the retry.

That is a different failure mode from float jitter. Something in the run occasionally diverges completely, which points at initialisation or at an unseeded stream in the rollout path rather than at accumulated rounding.

What it does not point at

Not the interpreter and not a dependency: the branch changed one YAML file, and main has been green on every run since 6 August. Not thread count, which ci.yml already pins to 1. Not the global RNG seeding, which conftest.py handles per test through set_seed.

Overlap with #22

"Polish: seeded rollout-buffer RNG and eval-freeze for the normalization wrappers" is aimed at exactly this area. If the rollout buffer was drawing from an unseeded stream, that would explain a run diverging while every RNG the test seeds looks correctly set — and it would explain both failures rather than one.

Worth checking whether #22 makes either test reproducible before deciding what the thresholds should be. If it does, this issue narrows to "the thresholds are still absolute" rather than "the runs are not repeatable".

[DenisDrobyshev]

Second observation of the same test, and it is not rare.

tests/test_recurrent.py::test_recurrent_ppo_learns_cartpole, on a branch whose entire diff is .github/dependabot.yml:

E       AssertionError: RecurrentPPO failed to learn CartPole (mean_return=20.4)
E       assert 20.4 > 60

Together with the earlier one, the record on pull-request branches today is:

Run	Result
dependabot-floors, first attempt	fail, mean 19.6
same run, re-run unchanged	pass
ignores-that-work	fail, mean 20.4

main has passed on its runs in the same window.

Three executions is not a rate, and I am not claiming one — the sample is small and drawn only from branches. But two failures out of three, both landing at random-policy level rather than near the threshold, is a different thing from an occasional tail.

What that suggests

If the run were merely noisy around a threshold, failures would cluster near 60. Both failures are at ~20, which is what a policy that never learned scores. So the run is not underperforming; it is bimodal — it either learns or it does not, and "does not" is common.

That points away from "the threshold is too tight" and toward something in the run genuinely diverging. Which makes #22 the interesting one: if the rollout buffer was drawing from an unseeded stream, an occasional total divergence is exactly the shape you would expect.

Worth measuring before changing the assertion: run this test twenty times on one commit and count the modes. If it is 2 in 3, the threshold is not the problem.

Practical effect meanwhile

It blocks unrelated work. #31 changes one YAML file and cannot merge because of this.