Skip to content

Honor seed=0 in iterate_batches - #1661

Merged
michalk8 merged 1 commit into
ml-explore:mainfrom
axiom-of-choice:fix-iterate-batches-seed-zero
Aug 5, 2026
Merged

Honor seed=0 in iterate_batches#1661
michalk8 merged 1 commit into
ml-explore:mainfrom
axiom-of-choice:fix-iterate-batches-seed-zero

Conversation

@axiom-of-choice

Copy link
Copy Markdown
Contributor

Closes #1660.

Summary

iterate_batches gated its seeding on if seed:, so an explicit seed=0 was silently ignored and batch order fell back to whatever state global numpy happened to be in. 0 is the default in mlx_lm.lora's CONFIG_DEFAULTS, which made the most common value the one that did not work.

-    if seed:
+    if seed is not None:
         np.random.seed(seed)

Evaluation

Measured before the change, passing the seed explicitly and drawing from numpy in between to see whether the seed actually isolates the sequence:

seed=42 isolated from global state: True
seed=0  honored                   : False   <-- `if seed:` treats 0 as unset

After the change both are True, and different seeds still produce different orders.

The new test asserts exactly that, for seed=0 and seed=42. It fails on main:

-  [25, 25, 25, 25, 25, 25, 25, 25]]
+  [33, 33, 33, 33, 33, 33, 33, 33]]
FAILED (failures=1)

and passes with the fix. Full suite:

python -m unittest tests.test_tuner_trainer tests.test_finetune
Ran 17 tests in 0.204s -- OK

Scope

Deliberately just the bug fix. Worth being clear about what this does not change, since #1660 could be read as a larger claim:

Batch order for a normal mlx_lm.lora run is already reproducible today. run() in lora.py calls np.random.seed(args.seed) before training, train() calls iterate_batches without a seed argument, and nothing in the CLI training path draws from numpy in between, so the permutations come from that seeded global state. This fix does not alter that path at all.

What it fixes is the case where seed is passed explicitly and happens to be 0, which is what you hit calling train() or iterate_batches directly from Python, and which is the documented parameter for this.

A second, larger question is whether train() should pass args.seed through to iterate_batches so batch order depends on an explicit seed rather than ambient global state. That would change batch order for a given seed relative to today, so I left it out. Happy to send it separately if you want it.

Risks

Low. The only behavior change is that seed=0 now seeds instead of being ignored. Anyone previously passing 0 was getting unseeded behavior by accident; they now get deterministic ordering, which is what the argument promises. No change for seed=None or any non-zero seed, and no change to the CLI path.

Pre-merge

Nothing. One-line change, no new dependencies, no config or docs updates needed.

Post-merge

Nothing required. If you'd like the train() passthrough discussed above, I can open it as a follow-up.

Environment: mlx-lm at e5baded, mlx 0.32.0, M2 Pro, macOS 26.5.2.

iterate_batches gated its seeding on `if seed:`, so an explicit seed of 0
was silently ignored and batch order fell back to whatever state global
numpy happened to be in. 0 is the default seed in mlx_lm.lora's
CONFIG_DEFAULTS, which made the most common value the one that did not work.

Use `if seed is not None:` instead. Add a regression test asserting a given
seed produces the same batch order regardless of other numpy use in between,
for both 0 and a non-zero seed. The test fails before this change.
@michalk8
michalk8 self-requested a review August 5, 2026 12:47

@michalk8 michalk8 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@michalk8
michalk8 merged commit 00aeaea into ml-explore:main Aug 5, 2026
2 checks passed
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.

iterate_batches ignores seed=0 due to if seed:, and 0 is the default seed

2 participants