Skip to content

fix(examples): refuse a split too small to score at setup, naming the size that works - #101

Open
emfdavid wants to merge 1 commit into
mainfrom
fix/empty-split-refusal
Open

emfdavid wants to merge 1 commit into
mainfrom
fix/empty-split-refusal

Conversation

@emfdavid

Copy link
Copy Markdown
Owner

Summary

Closes #87. Shrinking an example until the val split came up empty gave an error that named neither the cause nor the knob. #87 lists two instances; this PR fixes those plus a third found on the way:

path before after
examples.sdss.train_torch --n-plates 3 RuntimeError: empty dataset: cannot infer spectrum width from a width probe refused at setup: val has nothing to draw, "Use at least 512 samples (4 chunks of 128) -- --n-plates … (one plate is one chunk)"
bench.advection_sweep --n-steps 192 passes the parent's 3 * sample_chunk precheck, then the child refuses under a CalledProcessError argparse error before any child runs, naming 384 and the 24-step horizon
examples.microscopy with < 6 Z-planes (not in #87) trains a full run, then refuses in evaluate refused at setup, naming 6

One solver. #71's minimum-size solver was private to examples/advection. It now lives in examples/_splits.py, and advection, SDSS, microscopy and the sweep all call it. It works on geometry alone (split_by_chunk + drawable_samples, the same engine functions describe() uses), which is what lets the sweep apply the child's exact rule before any store exists. bench now imports the split fractions and horizon from examples.advection.data rather than restating them, the same way it already reads _BLOCK_CHUNKS from the engine.

A latent bug in #71's solver, fixed. Its stand-in geometry was ArrayGeometry(shape=(n, *geom.shape[1:]), …), which lengthened axis 0 and dropped sample_axis. On a middle-axis layout it split the wrong axis. For the IDR mask (Z-chunk 30 behind a T axis chunked 1) it named 30 samples where 180 are needed, so the advice itself would have left val empty. No example hit this yet, because only advection (axis 0) called it; microscopy (axis 2) would have been the first.

For reviewers

Each named minimum is checked against an answer the solver didn't produce:

The stricter sweep precheck still clears the documented default (4000 steps) for every config; the strictest, sample_chunk 256, needs 1536. A test pins this.

One behavior change worth a look: advection's check now runs before the InSituDataset is built and computes from the geometry, where #71 read ds.describe() afterwards. It uses the same engine functions and the #71 tests pass unchanged, but it no longer reads the numbers off the built dataset.

The minimum is solved in whole chunks, as in #71. So "at least N" is the smallest chunk-multiple that works, which matches the plate/plane knobs exactly.

Author attestation

  • I have reviewed every change in this PR, I can explain why each one is correct, and I
    have verified the claims made in this description.

Checklist

  • Tests added or updated. Each bug has a test that failed before the fix: the SDSS and microscopy refusal tests, the sweep precheck test, and the axis test (confirmed failing by restoring the old line).
  • uv run ruff check src tests bench examples, uv run ruff format --check src tests bench examples, uv run mypy src bench examples and uv run pytest -q are green
    locally (678 passed, 9 skipped; test_tf.py skipped because TensorFlow isn't installed in this env), plus uv run --extra docs mkdocs build --strict
  • Docstrings updated for the changed example entry points (reconstruct_dataset, segmentation_dataset)
  • A bullet added under ## Unreleased in CHANGELOG.md (states that it resolves Examples fail unhelpfully when the input is small enough to empty a split #87)
  • No load-bearing invariant is broken

🤖 Generated with Claude Code

… size that works

The SDSS example, the microscopy example and the advection sweep each failed late and
opaquely when an input was small enough to empty the val split: SDSS from a width probe,
microscopy only after training in `evaluate`, and the sweep's own precheck used a
`3 * sample_chunk` bound that ignored the 24-step horizon, so the child refused what the
parent had passed.

#71's solver moves to `examples/_splits.py` and all of them use it: the examples refuse
at setup from the engine's `split_by_chunk` + `drawable_samples`, and the sweep applies
the child's exact rule at argparse. The move also fixes a latent bug: the stand-in
geometry lengthened axis 0 rather than the sample axis, naming 30 samples for the IDR
mask layout where 180 are needed.

Closes #87

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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.

Examples fail unhelpfully when the input is small enough to empty a split

1 participant