Skip to content

Commit a9ae5f6

Browse files
committed
perf: reuse an already-correctly-capped dataset instead of re-simulating it
should_simulate's PYAUTO_SMALL_DATASETS=1 branch deleted and re-simulated every dataset unconditionally. That was correct when written, and the docstring said why: it "cannot know the capped dataset on disk was produced by the SAME cap, so it always regenerates". The SMALLDAT stamp (PyAutoNerves#153) removes exactly that limitation, so the branch was paying for a full simulation pass per dataset per smoke run across ~253 call sites, for datasets that were already correct. Reuse now requires positive evidence on BOTH counts: stamped SMALLDAT = T AND measuring exactly SMALL_DATASETS_SHAPE_NATIVE. The second half is the trap, and dropping it would be worse than not doing this at all. SMALLDAT = T means "capped at whatever the cap was when this was written", NOT "capped at today's cap". If SMALL_DATASETS_SHAPE_NATIVE is ever changed, every dataset on disk goes on claiming T at the old size, and reusing on the stamp alone would silently feed stale wrong-sized data to a run that asked for the new cap -- the same silent-stale-dataset bug the stamp was introduced to prevent, reintroduced through the opposite branch. Pinned by test. This is the mirror image of _stamp_contradicted_by_shape on the full-resolution branch and exists for the same reason: the stamp records the writer's environment, not a measured property of the data. Neither branch may treat it as unfalsifiable. Interferometer datasets deliberately never qualify and keep regenerating every run. Their data.fits is (n_visibilities, 2) -- shape fixed by the committed uv file, unchanged by the cap -- so shape cannot corroborate their stamp, and the alternative is trusting the stamp alone for precisely the family whose corruption is invisible. Stated in the docstring rather than left to fall out of the code. Everything without a readable top-level data.fits (JSON-only, datacubes nesting theirs in channel_XXX/, multi_dataset's prefixed names) also fails the check and regenerates, preserving today's behaviour for the families this cannot speak about. The existing test asserting the unconditional behaviour encoded the limitation being removed; rewritten, plus cases for each trap above.
1 parent 82b5d16 commit a9ae5f6

2 files changed

Lines changed: 130 additions & 8 deletions

File tree

autoarray/util/dataset_util.py

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,44 @@ def _stamp_contradicted_by_shape(dataset_path):
214214
)
215215

216216

217+
def _is_capped_at_the_current_cap(dataset_path):
218+
"""
219+
Returns True only when the dataset on disk was written by a capped run
220+
**and** was capped to the size in force right now.
221+
222+
Both halves are required, and the second is the one that is easy to drop.
223+
``SMALLDAT = T`` does not mean "capped at today's cap" -- it means "capped at
224+
whatever ``SMALL_DATASETS_SHAPE_NATIVE`` was when this file was written". If
225+
that constant is ever changed, every dataset already on disk goes on claiming
226+
``T`` at the old size, and reusing them on the stamp alone would silently
227+
feed stale, wrong-sized data to a run that asked for the new cap -- the exact
228+
class of silent-stale-dataset bug the stamp was introduced to prevent,
229+
reintroduced through the opposite branch.
230+
231+
This is the mirror image of :func:`_stamp_contradicted_by_shape` on the
232+
full-resolution branch, and it exists for the same reason: the stamp records
233+
the writer's *environment*, not a measured property of the data. Neither
234+
branch may treat it as unfalsifiable.
235+
236+
Interferometer datasets deliberately never satisfy this. Their ``data.fits``
237+
is ``(n_visibilities, 2)`` -- its shape is fixed by the committed uv file and
238+
does not change under the cap -- so shape cannot corroborate their stamp, and
239+
a capped interferometer dataset is regenerated on every run exactly as it was
240+
before. That is the conservative choice and it is deliberate: the alternative
241+
is trusting the stamp alone for precisely the family whose corruption is
242+
invisible.
243+
244+
Anything without a readable ``data.fits`` at the top level -- JSON-only
245+
datasets, datacubes nesting theirs in ``channel_XXX/``, multi_dataset's
246+
prefixed names -- also returns False and is regenerated, preserving today's
247+
behaviour for the families this cannot speak about.
248+
"""
249+
return (
250+
_small_datasets_stamp_on_disk(dataset_path) is True
251+
and _is_small_datasets_on_disk(dataset_path)
252+
)
253+
254+
217255
def should_simulate(dataset_path):
218256
"""
219257
Returns True if the dataset at ``dataset_path`` needs to be simulated.
@@ -223,10 +261,16 @@ def should_simulate(dataset_path):
223261
masks and grids to ``SMALL_DATASETS_SHAPE_NATIVE``. Both directions are
224262
checked:
225263
226-
- Entering the **small** regime, any existing dataset is deleted so the
264+
- Entering the **small** regime, an existing dataset is deleted so the
227265
simulator re-creates it at the reduced resolution, avoiding shape
228-
mismatches between full-resolution FITS on disk and the capped
229-
mask/grid.
266+
mismatches between full-resolution FITS on disk and the capped mask/grid.
267+
It is **kept** only when it is already capped to the size in force now --
268+
stamped ``SMALLDAT = T`` *and* measuring exactly
269+
``SMALL_DATASETS_SHAPE_NATIVE`` (:func:`_is_capped_at_the_current_cap`).
270+
Before the stamp this branch was unconditional because it had no way to
271+
tell a dataset produced by the same cap from one produced by a different
272+
run; it does now, and re-simulating an already-correct dataset is pure
273+
cost on every smoke run across the workspaces.
230274
- Entering the **full** regime, a dataset left behind by an earlier capped
231275
run is likewise deleted. Existence alone cannot distinguish the two, so
232276
the regime is taken from the ``SMALLDAT`` header card that
@@ -316,7 +360,9 @@ def should_simulate(dataset_path):
316360
that is true of weak lensing only.
317361
"""
318362
if os.environ.get("PYAUTO_SMALL_DATASETS") == "1":
319-
if Path(dataset_path).exists():
363+
if Path(dataset_path).exists() and not _is_capped_at_the_current_cap(
364+
dataset_path
365+
):
320366
shutil.rmtree(dataset_path)
321367

322368
return not Path(dataset_path).exists()

test_autoarray/util/test_dataset_util.py

Lines changed: 80 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ def test__env_set__non_square_above_cap__center_crops_to_16x16(monkeypatch):
113113
_on_disk_shape_native,
114114
_small_datasets_stamp_on_disk,
115115
_stamp_contradicted_by_shape,
116+
_is_capped_at_the_current_cap,
116117
SMALL_DATASETS_HEADER_KEY,
117118
)
118119

@@ -188,13 +189,88 @@ def test__small_regime__existing_full_dataset__is_deleted_and_resimulated(
188189
assert not dataset_path.exists()
189190

190191

191-
def test__small_regime__existing_small_dataset__is_still_deleted_and_resimulated(
192+
def test__small_regime__dataset_already_at_the_current_cap__is_kept(
192193
monkeypatch, tmp_path
193194
):
194-
# The small path is unconditional by design: it cannot know the capped
195-
# dataset on disk was produced by the SAME cap, so it always regenerates.
195+
# This branch used to be unconditional because it had no way to tell a
196+
# dataset produced by the SAME cap from any other. The stamp gives it one,
197+
# so an already-correct dataset is reused instead of re-simulated -- pure
198+
# cost otherwise, on every smoke run across ~253 call sites.
196199
monkeypatch.setenv("PYAUTO_SMALL_DATASETS", "1")
197-
dataset_path = _write_dataset(tmp_path / "dataset", SMALL_DATASETS_SHAPE_NATIVE)
200+
dataset_path = _write_dataset(
201+
tmp_path / "dataset", SMALL_DATASETS_SHAPE_NATIVE, stamp=True
202+
)
203+
204+
assert should_simulate(str(dataset_path)) is False
205+
assert (dataset_path / "data.fits").exists()
206+
207+
208+
def test__small_regime__stamped_capped_but_at_a_DIFFERENT_cap__is_regenerated(
209+
monkeypatch, tmp_path
210+
):
211+
# THE TRAP. `SMALLDAT = T` means "capped at whatever the cap was when this
212+
# was written", NOT "capped at today's cap". Reusing on the stamp alone
213+
# would silently feed stale wrong-sized data to a run that asked for a
214+
# different cap -- the same silent-stale-dataset bug the stamp exists to
215+
# prevent, reintroduced through the opposite branch.
216+
monkeypatch.setenv("PYAUTO_SMALL_DATASETS", "1")
217+
other_cap = (SMALL_DATASETS_SHAPE_NATIVE[0] * 2, SMALL_DATASETS_SHAPE_NATIVE[1] * 2)
218+
dataset_path = _write_dataset(tmp_path / "dataset", other_cap, stamp=True)
219+
220+
assert _small_datasets_stamp_on_disk(str(dataset_path)) is True # claims capped
221+
assert should_simulate(str(dataset_path)) is True # but not at THIS cap
222+
assert not dataset_path.exists()
223+
224+
225+
def test__small_regime__unstamped_legacy_dataset__is_regenerated(
226+
monkeypatch, tmp_path
227+
):
228+
# Every dataset written before the stamp landed is unstamped. Reuse requires
229+
# positive evidence, so these keep the old always-regenerate behaviour.
230+
monkeypatch.setenv("PYAUTO_SMALL_DATASETS", "1")
231+
dataset_path = _write_dataset(
232+
tmp_path / "dataset", SMALL_DATASETS_SHAPE_NATIVE, stamp=None
233+
)
234+
235+
assert should_simulate(str(dataset_path)) is True
236+
assert not dataset_path.exists()
237+
238+
239+
def test__small_regime__full_resolution_dataset__is_regenerated(monkeypatch, tmp_path):
240+
# A dataset stamped F is full resolution; the capped run needs a capped one.
241+
monkeypatch.setenv("PYAUTO_SMALL_DATASETS", "1")
242+
dataset_path = _write_dataset(tmp_path / "dataset", (180, 180), stamp=False)
243+
244+
assert should_simulate(str(dataset_path)) is True
245+
assert not dataset_path.exists()
246+
247+
248+
def test__small_regime__interferometer_dataset__is_always_regenerated(
249+
monkeypatch, tmp_path
250+
):
251+
# Deliberate and conservative. An interferometer data.fits is
252+
# (n_visibilities, 2) -- its shape is fixed by the committed uv file and does
253+
# not change under the cap -- so shape cannot corroborate its stamp. Rather
254+
# than trust the stamp alone for precisely the family whose corruption is
255+
# invisible, this family keeps regenerating every run, as before.
256+
monkeypatch.setenv("PYAUTO_SMALL_DATASETS", "1")
257+
dataset_path = _write_dataset(tmp_path / "dataset", (360, 2), stamp=True)
258+
259+
assert _small_datasets_stamp_on_disk(str(dataset_path)) is True
260+
assert should_simulate(str(dataset_path)) is True
261+
assert not dataset_path.exists()
262+
263+
264+
def test__small_regime__dataset_with_no_top_level_data_fits__is_regenerated(
265+
monkeypatch, tmp_path
266+
):
267+
# JSON-only datasets, datacubes nesting FITS in channel_XXX/, multi_dataset's
268+
# prefixed names: no readable data.fits means no positive evidence, so they
269+
# regenerate exactly as they did before.
270+
monkeypatch.setenv("PYAUTO_SMALL_DATASETS", "1")
271+
dataset_path = tmp_path / "dataset"
272+
dataset_path.mkdir()
273+
(dataset_path / "dataset.json").write_text("{}")
198274

199275
assert should_simulate(str(dataset_path)) is True
200276
assert not dataset_path.exists()

0 commit comments

Comments
 (0)