diff --git a/autoarray/util/dataset_util.py b/autoarray/util/dataset_util.py index ede4f03eb..068dd6a99 100644 --- a/autoarray/util/dataset_util.py +++ b/autoarray/util/dataset_util.py @@ -6,6 +6,14 @@ SMALL_DATASETS_SHAPE_NATIVE = (16, 16) SMALL_DATASETS_PIXEL_SCALES = 0.6 +# The FITS header card ``autonerves.fitsable.stamp_small_datasets_regime`` writes +# on every array the stack outputs. Deliberately duplicated here rather than +# imported: ``pyproject.toml`` floors autonerves at a release that predates the +# stamp, so an import would hard-fail against a legitimately-resolved older +# autonerves. Reading the card by name degrades to "absent" instead, which is +# exactly the fallback path below. Keep in sync with PyAutoNerves#153. +SMALL_DATASETS_HEADER_KEY = "SMALLDAT" + def cap_array_2d_for_small_datasets(array_2d, pixel_scales): """ @@ -97,6 +105,41 @@ def _on_disk_shape_native(data_path): return None +def _small_datasets_stamp_on_disk(dataset_path): + """ + Returns the small-datasets regime recorded in ``data.fits``'s header, as a + tri-state: ``True`` (written by a capped run), ``False`` (written at full + resolution), or ``None`` (no usable stamp -- unknown). + + ``None`` is returned for a missing file, an unreadable file, a file with no + ``SMALLDAT`` card, **and** a card whose value is not a genuine FITS boolean. + That last case is not pedantry: ``bool("F")`` is ``True`` in Python, so + coercing a hand-edited or third-party string card would invert the regime + and hand a ``True`` to a predicate that ends in ``shutil.rmtree``. A card + this code did not write is not a card this code can trust. + + Callers must treat ``None`` as "leave the dataset alone" and fall back to + :func:`_is_small_datasets_on_disk`. Unknown must never mean "full". + """ + from astropy.io import fits + + data_path = Path(dataset_path) / "data.fits" + + if not data_path.exists(): + return None + + try: + with fits.open(data_path) as hdu_list: + for hdu in hdu_list: + value = hdu.header.get(SMALL_DATASETS_HEADER_KEY) + if isinstance(value, bool): + return value + except Exception: + return None + + return None + + def _is_small_datasets_on_disk(dataset_path): """ Returns True if the dataset on disk at ``dataset_path`` was written by a @@ -128,6 +171,49 @@ def _is_small_datasets_on_disk(dataset_path): return _on_disk_shape_native(data_path) == SMALL_DATASETS_SHAPE_NATIVE +def _stamp_contradicted_by_shape(dataset_path): + """ + Returns True when ``data.fits`` claims ``SMALLDAT = T`` but its shape says it + cannot have been written by a capped run. + + The stamp and this predicate are not about the same thing, and that gap is + the whole reason this guard exists. ``stamp_small_datasets_regime`` records + *"the env var was set in the writing process"*. ``should_simulate`` acts on + *"this data is capped, therefore stale and disposable"*. The library itself + already makes those two diverge: + + - ``Kernel2D.from_gaussian`` passes ``respect_small_datasets=False`` + (``convolver.py``) because a kernel's shape is intrinsic to the + convolution operator, so a PSF written under the cap is full resolution; + - ``Interferometer.from_fits`` applies no cap at all; + - and any user converting real telescope data in a shell that exports + ``PYAUTO_SMALL_DATASETS=1`` -- the documented harness default -- stamps + ``T`` on genuinely full-resolution data. + + Every capped 2D image, by contrast, is rewritten to *exactly* + ``SMALL_DATASETS_SHAPE_NATIVE`` by ``Grid2D.uniform`` or ``Mask2D.circular``. + So a stamp of ``T`` on an image larger than the cap in **both** axes is a + self-contradiction, and a predicate ending in ``shutil.rmtree`` must resolve + a contradiction toward *keep*. + + Both axes, never either: interferometer ``data.fits`` is ``(n_visibilities, + 2)`` -- 108384 x 2 for the committed sdp81 dataset -- so an "either axis" + test would refuse to delete the one family the stamp exists to catch. Real + imaging (151x151, 209x209, 300x300) trips both and is protected. + + Unknown shape means not contradicted: this guard only ever *blocks* a + deletion, so failing to read the file must not silently protect a genuinely + stale dataset the stamp correctly identified. + """ + shape = _on_disk_shape_native(Path(dataset_path) / "data.fits") + + return ( + shape is not None + and shape[0] > SMALL_DATASETS_SHAPE_NATIVE[0] + and shape[1] > SMALL_DATASETS_SHAPE_NATIVE[1] + ) + + def should_simulate(dataset_path): """ Returns True if the dataset at ``dataset_path`` needs to be simulated. @@ -143,8 +229,11 @@ def should_simulate(dataset_path): mask/grid. - Entering the **full** regime, a dataset left behind by an earlier capped run is likewise deleted. Existence alone cannot distinguish the two, so - the regime is inferred from the data on disk - (``_is_small_datasets_on_disk``). + the regime is taken from the ``SMALLDAT`` header card that + ``autonerves.fitsable`` stamps into every FITS the stack writes + (``_small_datasets_stamp_on_disk``), falling back to inferring it from + the data's shape (``_is_small_datasets_on_disk``) for datasets written + before that stamp existed. That second check is what makes a local FAIL mean something. ``dataset/`` is gitignored in the workspaces, so CI clones fresh and always simulates, @@ -160,20 +249,71 @@ def should_simulate(dataset_path): if aa.util.dataset.should_simulate(dataset_path): subprocess.run([sys.executable, "scripts/.../simulator.py"], check=True) + Precedence + ---------- + The stamp wins over the shape heuristic, and the three states are not + interchangeable: + + - ``SMALLDAT = T`` -- delete, **unless the data contradicts the card**. The + stamp is preferred over the shape heuristic but it is not unfalsifiable: + it records that the env var was set at write time, which is not the same + proposition as "this array was capped" (see + :func:`_stamp_contradicted_by_shape`). Without that corroboration this + predicate would delete a full-resolution dataset the pre-stamp heuristic + explicitly refused to delete -- a strict weakening of the safety property + PyAutoArray#471 established. + - ``SMALLDAT = F`` -- **keep**, unconditionally, without consulting shape. + This also retires a false positive in the heuristic: a dataset that is + legitimately 16x16 at full resolution used to be deleted on every run. + - **absent** -- unknown, so fall back to the shape heuristic. Absence must + never be read as "full resolution": every dataset written before the + stamp landed is absent, and treating those as full would resurrect the + original bug. + + Interferometer datasets are covered by the stamp and were not covered + before: their visibility count is fixed by the committed uv file while the + real-space grid behind it is capped, so a capped run writes a ``data.fits`` + with *identical* ``NAXIS`` and different values. That fails silently -- no + shape mismatch, no assertion -- which is why a shape heuristic could never + reach it (PyAutoNerves#153). + Known gap --------- - The full-regime check reads ``data.fits``, so it covers imaging-style - datasets only. It cannot see a stale capped dataset whose corruption is not - visible in that file's shape: - - - point-source and weak-lensing datasets, which are JSON with no FITS; - - interferometer datasets, whose visibility count is fixed by the uv file - while the real-space grid behind it is capped, so the capped and full - files share a shape and differ only in values. - - Those regress to the previous existence-only behaviour rather than being - fixed here. Closing them needs the regime recorded at write time rather - than inferred at read time. + This reads ``/data.fits`` and nothing else, which covers + roughly 228 of the 253 ``should_simulate`` call sites in autolens_workspace. + The rest have no file of that name at that level and so get no verdict: + + - interferometer **datacube** datasets, whose FITS sit in ``channel_XXX/`` + subdirectories; + - **multi_dataset** datasets, which prefix the name (``{waveband}_data.fits``); + - **sample** datasets, which nest under ``dataset_N/``; + - the two FITS-less directories, ``dataset/weak/simple`` and + ``dataset/point_source/multiple_sources``, which a FITS-header stamp + cannot reach under any placement. + + All of those fail *safe*: no ``data.fits`` means no stamp, which means + unknown, which means keep. Nothing is deleted that should not be. + + Of those, only the first three can actually harbour the stale-capped-dataset + bug. The FITS-less pair, which look like the worst gap, are the least + urgent: ``dataset/weak/simple`` is regime-**invariant** -- nothing in its + write path reads ``PYAUTO_SMALL_DATASETS``, so a capped run and a full run + produce an identical ``dataset.json`` and there is nothing to detect -- and + ``dataset/point_source/multiple_sources``, which *is* regime-dependent, is + excluded from harness execution by ``config/build/no_run.yaml`` pending + PyAutoLens#480. Both of those facts will expire; see the follow-up. + + Widening the lookup is deliberately **not** done here. This predicate ends + in ``shutil.rmtree``, and every trap recorded in autolens_workspace_test#260 + was about a widened match hitting a file it should not have -- a bare + ``*.fits`` glob would delete every PSF-carrying dataset on every run. Growing + the reach of a destructive predicate is its own change, with its own review, + not a rider on the one that changes where its input comes from. + + Note that point-source datasets are **not** in this gap: they write a + top-level ``data.fits`` alongside their JSON and are covered normally. The + original issue text grouped them with weak lensing as "JSON with no FITS"; + that is true of weak lensing only. """ if os.environ.get("PYAUTO_SMALL_DATASETS") == "1": if Path(dataset_path).exists(): @@ -181,8 +321,19 @@ def should_simulate(dataset_path): return not Path(dataset_path).exists() - if Path(dataset_path).exists() and _is_small_datasets_on_disk(dataset_path): - shutil.rmtree(dataset_path) + if Path(dataset_path).exists(): + stamp = _small_datasets_stamp_on_disk(dataset_path) + + if stamp is True and not _stamp_contradicted_by_shape(dataset_path): + # Written by a capped run, on the writer's own authority -- and the + # data does not contradict it. + shutil.rmtree(dataset_path) + elif stamp is None and _is_small_datasets_on_disk(dataset_path): + # No stamp to trust, so fall back to inferring from shape. + shutil.rmtree(dataset_path) + # stamp is False -> known full resolution -> keep. + # stamp is True but contradicted by the data -> keep. The stamp is + # preferred over the shape heuristic, but it is not unfalsifiable. return not Path(dataset_path).exists() diff --git a/test_autoarray/conftest.py b/test_autoarray/conftest.py index 9dd03465c..505c5497d 100644 --- a/test_autoarray/conftest.py +++ b/test_autoarray/conftest.py @@ -317,3 +317,26 @@ def pytest_collection_modifyitems(config, items): ) if needs_nufftax: item.add_marker(skip_nufftax) + + +@pytest.fixture(autouse=True) +def _regime_independent_test_output(monkeypatch): + """ + Clear ``PYAUTO_SMALL_DATASETS`` for every test unless the test sets it. + + Since PyAutoNerves#153 every FITS the stack writes carries a ``SMALLDAT`` + card whose value tracks this env var at write time. Several tests write into + **tracked** fixture paths (14 of them across this repo and PyAutoNerves -- + a pre-existing pattern), so without this the bytes those tests produce + depend on the ambient environment: run the suite in a shell exporting + ``PYAUTO_SMALL_DATASETS=1`` -- which ``should_simulate``'s own docstring calls + the default for most harness runs -- and the suite passes but leaves the + working tree dirty. + + Pinning it here restores the property the stamp took away, that test output + is a function of the test and not of the shell, and does so in one place + rather than by rewriting every fixture-writing test. Tests that need a + regime set it with ``monkeypatch.setenv`` in their body, which runs after + this fixture and wins. + """ + monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) diff --git a/test_autoarray/structures/arrays/files/array/output_test/array.fits b/test_autoarray/structures/arrays/files/array/output_test/array.fits index 0cff0a8f7..d028d2d18 100644 Binary files a/test_autoarray/structures/arrays/files/array/output_test/array.fits and b/test_autoarray/structures/arrays/files/array/output_test/array.fits differ diff --git a/test_autoarray/util/test_dataset_util.py b/test_autoarray/util/test_dataset_util.py index 71fa9024f..f5b7a4476 100644 --- a/test_autoarray/util/test_dataset_util.py +++ b/test_autoarray/util/test_dataset_util.py @@ -111,10 +111,52 @@ def test__env_set__non_square_above_cap__center_crops_to_16x16(monkeypatch): should_simulate, _is_small_datasets_on_disk, _on_disk_shape_native, + _small_datasets_stamp_on_disk, + _stamp_contradicted_by_shape, + SMALL_DATASETS_HEADER_KEY, ) -def _write_dataset(dataset_path, shape, extra_files=()): - """Write a minimal dataset directory containing a `data.fits` of `shape`.""" +def _set_stamp(file_path, stamp): + """ + Force the ``SMALLDAT`` provenance card on an already-written FITS file. + + ``aa.output_to_fits`` stamps whatever regime is in force when it runs, which + is not always the regime a test needs the file to claim. The three values: + + - ``True`` / ``False`` -- overwrite the card, so a test can write a dataset + that *claims* a regime independently of the env it was written under. + + Every test of the READER forces the stamp this way rather than relying on + ``aa.output_to_fits`` to write one. That is deliberate layering, not + convenience: the stamp is written by PyAutoNerves, ``pyproject.toml`` + floors ``autonerves`` at a release that predates it, and that floor is + currently the newest release on PyPI -- so in any environment resolving + autonerves from PyPI the writer emits no card at all. Tests that leaned on + the writer would fail there, and would silently pass via the shape + fallback rather than exercising the stamp. This suite owns the reader; the + writer is tested in ``test_autonerves/test_fitsable.py``. + - ``None`` -- strip the card, producing a **legacy** file byte-equivalent to + one written before PyAutoNerves#153. This is what keeps the shape-based + fallback exercised; without it the fallback would silently become dead + code and every pre-stamp dataset on disk would lose its protection. + """ + from astropy.io import fits + + with fits.open(file_path, mode="update") as hdu_list: + for hdu in hdu_list: + if stamp is None: + hdu.header.pop(SMALL_DATASETS_HEADER_KEY, None) + else: + hdu.header[SMALL_DATASETS_HEADER_KEY] = stamp + + +def _write_dataset(dataset_path, shape, extra_files=(), stamp="auto"): + """ + Write a minimal dataset directory containing a `data.fits` of `shape`. + + ``stamp="auto"`` leaves whatever regime was in force at write time; any + other value is forced onto the header via :func:`_set_stamp`. + """ dataset_path.mkdir(parents=True, exist_ok=True) aa.output_to_fits( @@ -123,6 +165,9 @@ def _write_dataset(dataset_path, shape, extra_files=()): overwrite=True, ) + if stamp != "auto": + _set_stamp(dataset_path / "data.fits", stamp) + for name, file_shape in extra_files: aa.output_to_fits( values=np.ones(file_shape), @@ -158,15 +203,56 @@ def test__small_regime__existing_small_dataset__is_still_deleted_and_resimulated def test__full_regime__stale_small_dataset__is_deleted_and_resimulated( monkeypatch, tmp_path ): - # THE REGRESSION TEST. Before the fix this returned False and the capped - # FITS were loaded at full resolution. + # THE REGRESSION TEST. Before autolens_workspace_test#260 this returned + # False and the capped FITS were loaded at full resolution. + # + # The dataset is WRITTEN under the cap so it carries a truthful + # ``SMALLDAT = T``, then read back in the full regime -- the actual + # sequence that produced the bug. Writing it with the env unset would + # stamp it ``F`` and describe a dataset that cannot exist. monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) - dataset_path = _write_dataset(tmp_path / "dataset", SMALL_DATASETS_SHAPE_NATIVE) + dataset_path = _write_dataset( + tmp_path / "dataset", SMALL_DATASETS_SHAPE_NATIVE, stamp=True + ) assert should_simulate(str(dataset_path)) is True assert not dataset_path.exists() +def test__full_regime__stale_small_dataset__no_stamp__shape_fallback_still_deletes( + monkeypatch, tmp_path +): + # The fallback is NOT throwaway work: every dataset already on disk when + # the stamp landed is unstamped, and the stamp can do nothing for those. + # A legacy capped dataset must still be caught by its shape. + monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) + dataset_path = _write_dataset( + tmp_path / "dataset", SMALL_DATASETS_SHAPE_NATIVE, stamp=None + ) + + assert _small_datasets_stamp_on_disk(str(dataset_path)) is None + assert should_simulate(str(dataset_path)) is True + assert not dataset_path.exists() + + +def test__full_regime__full_dataset_at_cap_shape__stamp_keeps_it( + monkeypatch, tmp_path +): + # The stamp RETIRES a false positive in the shape heuristic. A dataset that + # is legitimately 16x16 at full resolution is indistinguishable from a + # capped one by shape alone, so the heuristic deleted it on every single + # run. A truthful ``SMALLDAT = F`` is the only thing that can save it -- + # and it must win WITHOUT the shape check getting a vote. + monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) + dataset_path = _write_dataset( + tmp_path / "dataset", SMALL_DATASETS_SHAPE_NATIVE, stamp=False + ) + + assert _is_small_datasets_on_disk(str(dataset_path)) is True + assert should_simulate(str(dataset_path)) is False + assert (dataset_path / "data.fits").exists() + + def test__full_regime__full_dataset__is_kept(monkeypatch, tmp_path): monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) dataset_path = _write_dataset(tmp_path / "dataset", (180, 180)) @@ -273,3 +359,114 @@ def test__on_disk_shape_native__is_row_column_ordered(tmp_path): dataset_path = _write_dataset(tmp_path / "dataset", (30, 50)) assert _on_disk_shape_native(dataset_path / "data.fits") == (30, 50) + + +def test__stamp_reader__non_boolean_card__is_unknown_not_true(monkeypatch, tmp_path): + # bool("F") is True in Python. A string card left by a hand-edit or a + # third-party tool must therefore NEVER be coerced: doing so would report + # "capped" for a file claiming the opposite and feed that to shutil.rmtree. + # Only a genuine FITS boolean counts; anything else is unknown. + monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) + dataset_path = _write_dataset(tmp_path / "dataset", (180, 180)) + _set_stamp(dataset_path / "data.fits", "F") + + assert _small_datasets_stamp_on_disk(str(dataset_path)) is None + assert should_simulate(str(dataset_path)) is False + assert (dataset_path / "data.fits").exists() + + +def test__stamp_reader__unreadable_and_missing__are_unknown(monkeypatch, tmp_path): + # "Unknown regime" must mean "leave it alone", never "delete". + monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) + + missing = tmp_path / "no_such_dataset" + assert _small_datasets_stamp_on_disk(str(missing)) is None + + corrupt = tmp_path / "corrupt" + corrupt.mkdir() + (corrupt / "data.fits").write_text("this is not a FITS file") + assert _small_datasets_stamp_on_disk(str(corrupt)) is None + assert should_simulate(str(corrupt)) is False + assert (corrupt / "data.fits").exists() + + +def test__interferometer_shaped_dataset__stamp_catches_what_shape_cannot( + monkeypatch, tmp_path +): + # THE CASE THIS TASK EXISTS FOR. An interferometer dataset's visibility + # count is fixed by the committed uv file while the real-space grid behind + # it is capped, so a capped run writes a data.fits with IDENTICAL NAXIS and + # different values. There is no shape mismatch and no assertion to trip -- + # it fails silently, which is strictly worse than the loud imaging failure. + # + # Shape is provably blind to it; the stamp provably is not. + monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) + dataset_path = _write_dataset(tmp_path / "dataset", (360, 2), stamp=True) + + # The heuristic that fixed the imaging case sees nothing wrong here. + assert _is_small_datasets_on_disk(str(dataset_path)) is False + # The stamp does. + assert _small_datasets_stamp_on_disk(str(dataset_path)) is True + assert should_simulate(str(dataset_path)) is True + assert not dataset_path.exists() + + +def test__psf_carrying_dataset__is_not_deleted_by_a_glob(monkeypatch, tmp_path): + # Guards the predecessor's trap: PSF kernels are legitimately tiny at full + # resolution, so anything keying on "the first FITS in the directory" + # instead of data.fits by name would delete every PSF-carrying dataset on + # every run. The stamp path must not reintroduce that. + monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) + dataset_path = _write_dataset( + tmp_path / "dataset", (180, 180), extra_files=(("psf.fits", (11, 11)),) + ) + + assert should_simulate(str(dataset_path)) is False + assert (dataset_path / "psf.fits").exists() + + +def test__stamp_true_on_a_full_resolution_image__is_contradicted_and_kept( + monkeypatch, tmp_path +): + # THE DATA-LOSS REGRESSION. A user converting real 300x300 telescope data in + # a shell exporting PYAUTO_SMALL_DATASETS=1 -- the documented harness + # default -- stamps T on full-resolution data. Acting on that stamp alone + # deletes their data, and the pre-stamp shape heuristic explicitly REFUSED + # to: a stamp-preferring rule must not be a strict weakening of the safety + # property PyAutoArray#471 established. + monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) + dataset_path = _write_dataset(tmp_path / "dataset", (300, 300), stamp=True) + + assert _small_datasets_stamp_on_disk(str(dataset_path)) is True + assert _is_small_datasets_on_disk(str(dataset_path)) is False # #471 said keep + assert _stamp_contradicted_by_shape(str(dataset_path)) is True + + assert should_simulate(str(dataset_path)) is False + assert (dataset_path / "data.fits").exists() + + +def test__contradiction_guard__needs_BOTH_axes_over_the_cap(monkeypatch, tmp_path): + # Both axes, never either. Interferometer data.fits is (n_visibilities, 2) -- + # 108384 x 2 for the committed sdp81 dataset -- so an "either axis" test + # would refuse to delete the exact family the stamp exists to catch. + monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) + + interferometer = _write_dataset(tmp_path / "interf", (108384, 2), stamp=True) + assert _stamp_contradicted_by_shape(str(interferometer)) is False + + imaging = _write_dataset(tmp_path / "imaging", (151, 151), stamp=True) + assert _stamp_contradicted_by_shape(str(imaging)) is True + + at_cap = _write_dataset(tmp_path / "at_cap", SMALL_DATASETS_SHAPE_NATIVE, stamp=True) + assert _stamp_contradicted_by_shape(str(at_cap)) is False + + +def test__contradiction_guard__unknown_shape_does_not_block_a_deletion( + monkeypatch, tmp_path +): + # The guard only ever BLOCKS a delete, so an unreadable file must not + # silently protect a dataset the stamp correctly identified as stale. + monkeypatch.delenv("PYAUTO_SMALL_DATASETS", raising=False) + + missing = tmp_path / "gone" + assert _stamp_contradicted_by_shape(str(missing)) is False