Repository navigation
bug: should_simulate rmtree's committed, gitignore-allowlisted datasets under PYAUTO_SMALL_DATASETS=1 #470
Description
Overview
should_simulate deletes tracked, version-controlled data and replaces it with capped-simulator output. This is independent of the stale-dataset bug it was found alongside, and no variant of that fix addresses it.
autoarray/util/dataset_util.py, small-datasets branch:
if os.environ.get("PYAUTO_SMALL_DATASETS") == "1":
if Path(dataset_path).exists():
shutil.rmtree(dataset_path)The rmtree is unconditional. It does not ask whether the directory holds generated data or committed data.
The live case
In autolens_workspace_test:
.gitignore:13reads!dataset/point_source/simple/**— an explicit allowlist exception, i.e. this directory is deliberately committed.git ls-filesconfirms three tracked JSON files under it (point_dataset_positions_only.json,point_dataset_with_fluxes_and_time_delays.json,tracer.json). The allowlist comments describe this class as real/external data that must never be purged.scripts/point_source/visualization/visualization.py:39callsshould_simulate("dataset/point_source/simple"), and the script declares onlyENV: real_plots(line 22), so it keepsPYAUTO_SMALL_DATASETS=1under the smoke profile defaults.
So an ordinary smoke run deletes committed data. It is then re-simulated through PointSolver.solve, which under the cap short-circuits to a fixed position pair (PyAutoLens autolens/point/solver/point_solver.py:119) — so the replacement content is not merely lower-resolution, it is degenerate.
Seven full-regime scripts then read that same directory, one of them in smoke_tests.txt.
Severity
The deletion is recoverable (git checkout -- dataset/point_source/simple) and shows up as a dirty working tree, so this is not data loss in the unrecoverable sense. But:
- it silently violates the invariant the
.gitignoreallowlist exists to express; - it leaves developers with an unexplained dirty tree after a routine smoke run;
- combined with the seven full-regime readers, it is the same mixed-regime collision class as feat: add title_prefix support to subplot_imaging_dataset #260, in a dataset family the shape-based fix is structurally blind to (JSON, no FITS).
Suggested direction
Make the destructive branch respect committed data rather than assuming every dataset directory is disposable. Options worth weighing:
- Skip
rmtreefor tracked paths — cheap to check, but puts a git dependency in a library utility, which is the wrong layer. - Have the workspace declare the exception — give
point_source/visualization/visualization.pyafull_datasets-style profile entry so it never enters the small regime against committed data. Narrowest fix, but leaves the general footgun armed for the next allowlisted dataset. - Regime path separation — capped runs write to a separate path so the two regimes never share a directory. This removes the collision rather than detecting it, and would also close feat: add title_prefix support to subplot_imaging_dataset #260's remaining gaps, but needs a path-rewrite layer in the IO with real sharp edges (committed read-only inputs like
uv_wavelengths/sma.fitsneed read-fallback, plot outputs, absolute paths).
(2) unblocks the immediate case; (3) is the architecturally correct shape if this class of bug keeps recurring.
Context
- Root-cause investigation this was found during: fix: jax_grad assertions fail locally but pass in CI autolens_workspace_test#260
- Related follow-up on recording the regime at write time: feat: stamp the small-datasets regime into FITS headers at the writer funnel PyAutoNerves#153
Activity
Jammy2211 commentedon Aug 22, 2026
Design call taken: option 2. Fix is open as autolens_workspace_test#264 — it lands in the workspace, so no PyAutoArray diff.
Why option 2, not 1 or 3
I swept every workspace's
!dataset/**allowlist against everyshould_simulatecall site that does not releasePYAUTO_SMALL_DATASETS. Exactly one at-risk site exists in the organism — thevisualization.pythis issue names:autolens_workspace_testautolens_workspaceautogalaxy_workspaceautocti_workspacedataset/<name>/imaging_ci/…, never the committeddataset/overview)The issue framed option 2 as leaving "the general footgun armed for the next allowlisted dataset", and option 3 as correct "if this class keeps recurring". The sweep says it does not recur — the class is a population of one — and six sibling scripts against this very dataset already declare
full_datasets, so option 2 restores an established convention rather than papering over a library flaw. Option 1 puts a git dependency inside a library IO utility, which this issue already calls the wrong layer.Two premises in the issue have moved
1. The
rmtreeis no longer unconditional. #471 guards it with_is_capped_at_the_current_cap. The defect survives anyway, because that helper reads<dataset>/data.fitsand this directory is JSON-only —autolens_workspace_test/.gitignorelistsdata.fitsunder "Generated artifacts — never check in", so no stamp can exist. The helper's own docstring names this exit: "Anything without a readabledata.fitsat the top level — JSON-only datasets … also returns False and is regenerated." Reproduced on currentmainbefore fixing: oneshould_simulatecall, all three tracked JSONs gone.2. "Seven full-regime scripts, one in
smoke_tests.txt" overstates the exposure. Of the 9 scripts touching this dataset, 6 declarefull_datasets, andmodeling_visualization_jit.py'sreal_outputis a superset token that also releases the cap. Thesmoke_tests.txtentry (jax_likelihood/point.py) is among the safe ones.One thing the issue understated
The degenerate data persists across scripts. Because the re-simulated directory still has no
data.fits, a later full-regime reader'sshould_simulategets stampNone, finds no shape to check, and keeps it. CI clones fresh so git restores it each run; locally, one run poisons the next — the same local-vs-CI asymmetry autolens_workspace_test#260 was about, in a family the shape-based fix is structurally blind to.Follow-up
should_simulate's docstring currently states that point-source datasets are not in the known gap because they "write a top-leveldata.fitsalongside their JSON". True inautolens_workspace, false inautolens_workspace_test— it reassures the reader about the exact case this issue is about. Correcting it, plus extendingPyAutoHands'check_dataset_allowlist.pyto catch this class automatically, is filed as a separate task (blocked:PyAutoHandsis claimed by another active task).Note on this issue's home: #470 is on PyAutoArray but the fix is entirely in
autolens_workspace_test. Kept here as the tracking issue rather than opening a duplicate; happy to transfer if preferred. The docstring half of the follow-up will touch PyAutoArray.docs(should_simulate): the point-source "not in this gap" note is rep…
Jammy2211 commentedon Aug 22, 2026
Closing — both halves have now shipped.
full_datasetson the one offending script220148a90f108ff50d28cThe defect —
should_simulatedeleted the committed, allowlist-protecteddataset/point_source/simpleon any capped run, replacing three tracked JSON files with degenerate output (PointSolver.solveshort-circuits to a fixed position pair under the cap). #471's stamp guard couldn't reach it: it reads<dataset>/data.fits, and that directory is JSON-only.The design call was option 2, taken on evidence rather than by default. An organism-wide sweep found exactly one at-risk call site, and six sibling scripts against the same dataset already declared
full_datasets— so the class was a population of one and the fix restored a convention, rather than papering over a library flaw that options 1 and 3 would have required a library diff to address.The footgun is now closed by a check, not by convention.
check_dataset_allowlistfails apre_buildrun when ashould_simulatecall site that has not releasedPYAUTO_SMALL_DATASETSwould delete git-tracked files.One finding from building that guard is worth recording here, because it is the trap anyone revisiting this area will hit. The natural predicate — "does the resolved path sit under an
!dataset/...allowlist prefix?" — is wrong. It produced six release-blocking failures in a workspace that commits documentation images directly in a dataset section while its scripts regenerate sibling subdirectories holding nothing tracked; deleting those destroys nothing. The correct invariant is "rmtree(path)would delete git-tracked files". Allowlist membership and deletion risk are not the same property, and only the second one is what this issue was ever about.Reopen if the class resurfaces in a family the guard's AST resolver can't reach — it reports and skips anything it cannot evaluate rather than guessing, and prints the skipped count so a partial sweep can't read as full coverage.