Repository navigation
Conversation
… 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
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
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.
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:
examples.sdss.train_torch --n-plates 3RuntimeError: empty dataset: cannot infer spectrum widthfrom a width probebench.advection_sweep --n-steps 1923 * sample_chunkprecheck, then the child refuses under aCalledProcessErrorexamples.microscopywith < 6 Z-planes (not in #87)evaluateOne solver. #71's minimum-size solver was private to
examples/advection. It now lives inexamples/_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 functionsdescribe()uses), which is what lets the sweep apply the child's exact rule before any store exists.benchnow imports the split fractions and horizon fromexamples.advection.datarather than restating them, the same way it already reads_BLOCK_CHUNKSfrom 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 droppedsample_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:
tests/test_example_splits.pysolves the IDR-mask geometry with Z in the middle and again with the same data laid out Z-first, and both must name 180. With the old construction put back, both cases fail.The stricter sweep precheck still clears the documented default (4000 steps) for every config; the strictest,
sample_chunk256, needs 1536. A test pins this.One behavior change worth a look: advection's check now runs before the
InSituDatasetis built and computes from the geometry, where #71 readds.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
have verified the claims made in this description.
Checklist
uv run ruff check src tests bench examples,uv run ruff format --check src tests bench examples,uv run mypy src bench examplesanduv run pytest -qare greenlocally (678 passed, 9 skipped;
test_tf.pyskipped because TensorFlow isn't installed in this env), plusuv run --extra docs mkdocs build --strictreconstruct_dataset,segmentation_dataset)## UnreleasedinCHANGELOG.md(states that it resolves Examples fail unhelpfully when the input is small enough to empty a split #87)🤖 Generated with Claude Code