fix(crf): the batch order no longer replays the split's shuffle - #231
Merged
Conversation
`resolve_split` seeds a generator from `seed`, and on both the corpus-split and held-out-batch paths it shuffles an array of the *same length* the epoch loop goes on to shuffle. `CrfTrainer.train` then opened a second `default_rng(seed)`, which replayed that generator's first draw exactly: epoch 1 trained on `pi(pi(train))` -- a batch order determined by the split rather than independent of it -- and every later epoch was the split stream shifted by one. Nothing about a run looks wrong when this happens. The order is still a permutation, the loss still falls, and the seed still reproduces. That is why it gets a test that names the property rather than a comment: the pre-fix first-epoch order is bit-identical to the split replay, and the test fails against it. `epoch_order_rng` spawns a distinct stream from the same seed, so runs stay reproducible and the two orders are independent. Skipping a single draw would have fixed epoch 1 and left the same defect one epoch further in, so the test checks several epochs deep. Found while auditing the ported trainer against escapepod-models' train_ctc.py for a systematic loss difference. The two turned out to be the same computation on the same data with the same init -- the batch-order stream was the only difference between them, and varying it alone moves a 32-epoch run's final training loss by more than 2x on this corpus.
jayhesselberth
added a commit
that referenced
this pull request
Aug 26, 2026
Minor rather than patch: new capability throughout, and two behaviour changes -- one confined to CRF training, one to how a corpus that cannot supply `signal_kmer` is handled. The release is the second half of the CTC-CRF port plus ONNX export: - `leech.crf.evaluate` (#224) -- decode a corpus, match to references by edit distance, report per group. The generic half of evaluation; what a panel is stays with whatever defines the panel. - ONNX export for the classifier arms and the CRF encoder (#217, #222), dynamo exporter at opset 18, each with a contract sidecar and a round-trip check against torch across the serialization boundary. - `leech model train-crf` (#219) -- the CLI for the trainer, plus the corpus builder (`plan_corpus`/`build_corpus`) and `CrfTrainer` itself. - The signal-level k-mer encoding now comes from escapepod-signal (#222) rather than being held in a cdylib no Rust consumer could link. Two behaviour changes, both worth reading before upgrading: `signal_kmer` no longer degrades quietly (#230/#232). The encoding is decided from the whole corpus rather than chunk 0, an encoding named on the command line is no longer substituted, and the config records what the run actually used. This one is coupled to the ONNX work above and is why the release waited for it: the contract is derived from the config and exists so a non-Python consumer can trust the input spec, so a config that misstates its encoding is now refused at export rather than published. CRF batch order (#231). `CrfTrainer.train` re-seeded `default_rng(seed)` and replayed the permutation `resolve_split` had already drawn, so epoch 1 trained on `pi(pi(train))`. Fixed, which means a given seed now sees different batches -- numbers from a seed will not reproduce against 0.8.0. Batch order alone moves a 32-epoch run's final training loss by more than 2x, so a seed is one draw from that spread, not a fixed point. The CRF trainer was validated against the implementation it was ported from over six paired seeds: balanced recall differs by -0.17pp +/- 0.28pp, sign test p = 0.688, against a within-arm seed range of 0.71pp. Also in this commit, not from the PRs: - README listed neither `leech model train-crf` (a shipped command missing from the CLI table) nor ONNX export at all, including the single-BCE-logit contract point that makes a misread graph silently wrong. - CLAUDE.md said "feature-complete (v0.7.0)" while listing CRF and ONNX. - CHANGELOG's Unreleased section had accumulated three separate `### Added` headings from different PRs; consolidated to one Added/Changed/Fixed set. Full suite 1512 passed, 44 skipped. Docs build clean. Both lockfiles verified against their manifests (`cargo metadata --locked`, `uv lock --check`).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
resolve_splitseeds a generator fromseed, and on both the corpus-split and held-out-batch paths it shuffles an array of the same length the epoch loop goes on to shuffle.CrfTrainer.trainthen opened a seconddefault_rng(seed)— which replays that generator's first draw exactly.So epoch 1 trained on
π(π(train)): a batch order determined by the split shuffle rather than independent of it, with every later epoch the split stream shifted by one.Why it needed a test rather than an eye
Nothing about a run looks wrong when this happens. The order is still a permutation, the loss still falls, and the seed still reproduces. The property is only visible if you state it:
epoch_order_rngspawns a distinct stream from the same seed, so runs stay reproducible and the two orders are independent. Skipping a single draw would have fixed epoch 1 and left the same defect one epoch further in, sotest_the_stream_is_not_merely_offset_from_the_split_streamchecks several epochs deep.How it was found
Auditing the ported trainer against escapepod-models'
train_ctc.py, which had been running ~1.4x lower final training loss over three paired seeds. The two turned out to be the same computation: identical corpus, identical split (107,878/107,878 test reads shared), identical architecture (28 tensors, 519,880 parameters), identical init seeding, and a step-for-step identical loop body. The batch-order stream was the only difference between them.Running one loop body twice with only the RNG stream varied reproduced a gap larger than the one between the arms:
That is batch-order variance, not a port defect — consistent with
train_ctc.py's own note that one seed reached 0.0045 where another reached 0.0072 on the same split.Compatibility
This changes which batches a given seed sees, so numbers from a seed will not reproduce across this release. Treat a seed as one draw from that spread rather than a fixed point.