|
| 1 | +- issue: none — shipped directly as a PR (small, single-repo follow-up of the closed PyAutoNerves#153) |
| 2 | +- completed: 2026-08-22 |
| 3 | +- library-pr: https://github.com/PyAutoLabs/PyAutoArray/pull/476 (merged dc0c273) |
| 4 | +- workspace-pr: none — no workspace change needed |
| 5 | + |
| 6 | +`should_simulate`'s `PYAUTO_SMALL_DATASETS=1` branch deleted and re-simulated |
| 7 | +**every** dataset unconditionally, paying a full simulation pass per dataset per |
| 8 | +smoke run across ~253 call sites for datasets that were already correct. That was |
| 9 | +correct when written and the docstring said why: it "cannot know the capped |
| 10 | +dataset on disk was produced by the SAME cap". The `SMALLDAT` stamp |
| 11 | +(`complete/2026/08/small-datasets-regime-stamp.md`) removed that limitation. |
| 12 | + |
| 13 | +**THE FIX IS NOT THE ONE-LINER THE PARENT RECORD PREDICTED.** That record — and |
| 14 | +the filed prompt's parent framing — said `if stamp is not True:` was a cheap fix. |
| 15 | +It is not, and shipping it would have been worse than doing nothing. |
| 16 | + |
| 17 | +`SMALLDAT = T` means "capped at whatever `SMALL_DATASETS_SHAPE_NATIVE` was when |
| 18 | +this file was written", NOT "capped at today's cap". If that constant ever |
| 19 | +changes, every dataset on disk goes on claiming `T` at the old size, and reusing |
| 20 | +on the stamp alone silently feeds stale wrong-sized data to a run that asked for |
| 21 | +the new cap — the exact silent-stale-dataset bug the stamp exists to prevent, |
| 22 | +reintroduced through the opposite branch. |
| 23 | + |
| 24 | +Reuse therefore requires BOTH: stamped `T` **and** shape `== SMALL_DATASETS_SHAPE_NATIVE` |
| 25 | +(`_is_capped_at_the_current_cap`). |
| 26 | + |
| 27 | +**This is the mirror image of `_stamp_contradicted_by_shape`** on the |
| 28 | +full-resolution branch, and exists for the same reason, which is the durable |
| 29 | +lesson from this pair of tasks: **the stamp records the writer's ENVIRONMENT, not |
| 30 | +a measured property of the data.** Neither branch may treat it as unfalsifiable. |
| 31 | +Any future consumer of `SMALLDAT` must corroborate it before acting destructively |
| 32 | +or before skipping work on its authority. |
| 33 | + |
| 34 | +**TRAPS** |
| 35 | +- Reuse on the stamp alone is the bug. Both halves are load-bearing. |
| 36 | +- Interferometer datasets deliberately NEVER qualify: `data.fits` is |
| 37 | + `(n_visibilities, 2)`, its shape fixed by the committed uv file and unchanged by |
| 38 | + the cap, so shape cannot corroborate the stamp. Trusting the stamp alone for |
| 39 | + precisely the family whose corruption is invisible is the wrong trade. Written |
| 40 | + into the docstring, not left to fall out of the code. |
| 41 | +- Anything with no readable top-level `data.fits` (JSON-only, datacubes nesting |
| 42 | + theirs in `channel_XXX/`, multi_dataset's prefixed names) fails the check and |
| 43 | + regenerates, preserving prior behaviour for the families this cannot speak about. |
| 44 | +- The pre-existing test asserting unconditional deletion ENCODED the limitation |
| 45 | + being removed. It had to be rewritten, not deleted — a test that fails because |
| 46 | + the limitation it documents is gone is a signal, not an obstacle. |
| 47 | + |
| 48 | +**Behaviour: exactly one row changes.** A dataset capped at the current cap is |
| 49 | +reused; different-cap-stamped, unstamped legacy, full-resolution, interferometer |
| 50 | +and no-`data.fits` all regenerate as before. Reuse requires positive evidence and |
| 51 | +everything else fails to provide it. |
| 52 | + |
| 53 | +Tests: 1106 passed / 0 failed, green with `PYAUTO_SMALL_DATASETS=1` exported AND |
| 54 | +unset, tree clean both ways. CI green on 3.12 / 3.13 / nojax. |
| 55 | + |
| 56 | +**Gate note.** Heart was not consulted — no PyAutoHeart checkout in this |
| 57 | +web-github session; the documented per-repo suite fallback was used, and CI |
| 58 | +subsequently agreed. |
| 59 | + |
| 60 | +## Original prompt |
| 61 | + |
| 62 | +# should_simulate's capped branch re-simulates every dataset, ignoring the stamp it now has |
| 63 | + |
| 64 | +Type: maintenance |
| 65 | +Target: libraries |
| 66 | +Repos: |
| 67 | +- @PyAutoArray |
| 68 | +Difficulty: small |
| 69 | +Autonomy: supervised |
| 70 | +Priority: medium |
| 71 | +Status: formalised |
| 72 | + |
| 73 | +Split out of PyAutoNerves#153 on 2026-08-22 (`complete/2026/08/small-datasets-regime-stamp.md`), |
| 74 | +which added the `SMALLDAT` regime stamp and deliberately did not touch this branch. |
| 75 | + |
| 76 | +`autoarray/util/dataset_util.py should_simulate` still does this when |
| 77 | +`PYAUTO_SMALL_DATASETS=1`: |
| 78 | + |
| 79 | +```python |
| 80 | +if os.environ.get("PYAUTO_SMALL_DATASETS") == "1": |
| 81 | + if Path(dataset_path).exists(): |
| 82 | + shutil.rmtree(dataset_path) |
| 83 | + return not Path(dataset_path).exists() |
| 84 | +``` |
| 85 | + |
| 86 | +Unconditional. Every smoke run deletes and re-simulates **every** dataset, even |
| 87 | +one already written by a capped run at the same cap. |
| 88 | + |
| 89 | +That was correct when written — the docstring says so explicitly: *"The small |
| 90 | +path is unconditional by design: it cannot know the capped dataset on disk was |
| 91 | +produced by the SAME cap, so it always regenerates."* The stamp removes exactly |
| 92 | +that limitation. `SMALLDAT = T` now says the writer capped it. |
| 93 | + |
| 94 | +`if stamp is not True:` is the shape of the fix. The saving is one full |
| 95 | +simulation pass per dataset per smoke run, across ~253 `should_simulate` call |
| 96 | +sites in autolens_workspace. |
| 97 | + |
| 98 | +## The trap that makes this not a one-liner |
| 99 | + |
| 100 | +**A stamp of `T` does not mean "capped at the cap size in force now."** It means |
| 101 | +"capped at whatever `SMALL_DATASETS_SHAPE_NATIVE` was when it was written." If |
| 102 | +that constant ever changes, every dataset on disk still claims `T` while being |
| 103 | +the wrong size, and skipping regeneration would silently reuse it — the same |
| 104 | +class of silent-stale-dataset bug the stamp exists to prevent, reintroduced |
| 105 | +through the other branch. |
| 106 | + |
| 107 | +So the reuse condition is not `stamp is True` alone. It needs the on-disk shape |
| 108 | +to also match the *current* cap, which `_on_disk_shape_native` already provides |
| 109 | +and `_is_small_datasets_on_disk` already compares with `== SMALL_DATASETS_SHAPE_NATIVE`. |
| 110 | +Reuse only when the stamp says capped **and** the shape matches today's cap; |
| 111 | +anything else regenerates. |
| 112 | + |
| 113 | +Note this is the mirror image of `_stamp_contradicted_by_shape` on the full-regime |
| 114 | +branch, and for the same reason: the stamp records the writer's environment, not |
| 115 | +a property of the data. Neither branch should treat it as unfalsifiable. |
| 116 | + |
| 117 | +Interferometer datasets are shape-invariant under the cap, so a shape check |
| 118 | +cannot corroborate them. Decide explicitly whether they reuse on the stamp alone |
| 119 | +or always regenerate — do not leave it to fall out of the code. |
| 120 | + |
| 121 | +## Suggested scope |
| 122 | + |
| 123 | +1. Reuse a capped dataset only when the stamp says `T` **and** the shape matches |
| 124 | + the current cap. Everything else regenerates, as today. |
| 125 | +2. Take the interferometer decision explicitly and write it in the docstring. |
| 126 | +3. Test both directions: same-cap dataset is reused; a dataset stamped `T` at a |
| 127 | + different shape is regenerated. |
| 128 | +4. Correct the docstring paragraph that says the small path cannot know. |
0 commit comments