Persist PSF products and add the two campaign-level merges (exp_persist, star_cat_merge, final_cat_merge) - #879
Merged
Merged
Conversation
cailmdaley
marked this pull request as ready for review
September 3, 2026 00:16
This was referenced Sep 5, 2026
Merged
Contributor
Author
|
transferring @martinkilbinger's comment on star/PSF products here: |
persist_exp.py copies one exposure's named PSF products off /scratch onto the persistent root and records what went, with sizes. The threat it answers is the 60-day purge, not clean_exposure: run_dir is scratch and products_dir is /project, so the only way a per-exposure product outlives its campaign is to leave the filesystem. Exempting files from reclamation would not have done it. The search is recursive beneath the PSF chain's four module output dirs, because setools writes into mask/, rand_split/, new_cat/, plot/ and stat/ rather than flat -- so the config's patterns stay plain file names and the layout stays ours. A pattern that matches nothing is a recorded warning (setools rejects sparse CCDs); nothing matching at all is a failure, since a green manifest over an empty copy is what would let reclamation delete an unsaved exposure. config.yaml's persist_exp: defaults to validation_psf-*.fits -- the psfex_interp VALIDATION catalogue, the rho/tau statistics input, the minimum. The opt-in candidates are documented there with what each buys; sizes are still to be measured. PSFEx residuals and XML are not candidates as the chain stands: the committed default.psfex sets CHECKIMAGE_TYPE NONE and WRITE_XML N. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One rule per exposure, output = ONE manifest on the persistent root at <products_dir>/exp/<shard>/<exp>/manifests/exp_persist.json. Not a directory() output: what we want written down is which files were copied and how big each was, and a directory attests only that a directory exists. Byte-stable, so a no-op rerun does not move an mtime clean_exposure reads. Its own rule rather than a cp on the end of exp_psf, and that is the whole point: the keep list rides on params, so adding a pattern reruns seconds of copying instead of four hours of PSF fitting per exposure. A localrule, by the arithmetic that made exp_star_cat one -- a few MB of cp, ~20k of them at DR6 scale, each shorter than the scheduling latency that would submit it. The mid-chain grouping constraint does not bite: its neighbours are exp_psf (too heavy to fuse) and clean_exposure (local already). clean_exposure gains the manifest as an input, so a store is never reclaimed before its keepers have left scratch -- conditional only on there being a keep list, since "keep nothing" must not become a dependency on a rule that would fail for having nothing to copy. rule all requests the persist manifests DIRECTLY, not only through clean_exposure: the purge takes the store whether or not clean: is on, so hanging the copy off reclamation alone would lose everything in a clean:false campaign. Cleaned exposures are excluded -- their exp_psf manifest is gone, so asking would rebuild the chain from VOS, and a tombstone already means the copy happened. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… omits it run_report disk-scans the scratch run_dir, and exp_persist's manifest is the one exposure manifest that lives on products_dir instead -- the placement that makes it survive clean_exposure. Listed in EXP_STAGES it would read as "not run" for every exposure in the campaign, so it is deliberately absent, with the reason on the line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e copies Inodes, not bytes, bind on /project (~1 M-file group quota): smk-m2 measured ~200 loose files per exposure with all candidates on — 25k for 64 tiles, ~2 M at DR6 scale, for 7 GB. persist_exp.py now writes <products_dir>/exp/<shard>/ <exp>/psf/<exp>.tar (uncompressed, flat members, deterministic: ownership zeroed, sorted, tmp-cmp-mv so a no-op rerun keeps the mtime) and the manifest lists every member. Manifest path, rule wiring and params are unchanged. config.yaml's candidate table carries the smk-m2 per-exposure sizes; psfex_cat and star_stat are marked unmeasured (no live store held them). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qtLUV3bVLPV5p6un7aTFR
An orphaned tar.tmp on /project is an inode nothing revisits — the leak the tar design exists to avoid. try/finally around both tmp writes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qtLUV3bVLPV5p6un7aTFR
develop no longer has exp_star_cat / star_catalogue (PR #847); the three comments that cited exp_star_cat as the localrule precedent now cite clean_exposure, which makes the same argument. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
cailmdaley
force-pushed
the
feat/persist-exp-products
branch
from
September 9, 2026 13:00
0d7c313 to
d91da36
Compare
Every rule in this workflow was per unit, and the two products downstream analysis actually opens are per campaign. So a run ended one file short on each side and both merges were a manual pass afterwards. These are the last links of chains the workflow already had. star_cat_merge stacks every exposure's every CCD's validation_psf into one <products_dir>/full_starcat-0000000.fits — the rho/tau statistics input, at the path sp_validation hardcodes. It reads the members straight out of the per-exposure tars exp_persist wrote (tarfile + BytesIO); unpacking ~800k files to merge them would defeat the tar's whole purpose. The stacking is MergeStarCatPSFEX, the class the old merge_starcat_runner called, so the column list keeps exactly one definition. That class gains one thing: an entry may be [fileobj, name] rather than [path], fits.open taking the first element and the CCD_NB regex the last — the same string for the runner's one-element entries. final_cat_merge collects every ready tile's final_cat into <products_dir>/final_cat_<campaign>.hdf5: one dataset per tile under a group named for the campaign, the final_cat.param columns, an n_tiles attribute. That schema is sp_validation's reader's, so it is fixed. The COLUMN EXTRACTION reuses scripts/python/create_final_cat.py (read_param_file, read_data, copy_data) so the column grammar keeps one definition; the file is written here, because that script's own discovery walks a directory layout this workflow does not have and groups by a unit ShapePipe v2 has dropped. Two places where the reference implementation is not reproducible are pinned down at the call site rather than copied: copy_data leaves every non-requested column as uninitialised memory, and read_param_file's column order varies with the process hash seed. bin/sp now snapshots the repo's scripts/ so the campaign pins that file like everything else it runs. Neither merge puts its input paths in its shell: ~20k of them is an order of magnitude over Linux's 128 KiB MAX_ARG_STRLEN for one argv entry. Each job is handed the two small files the Snakefile itself started from — the tile list and the run index — and DERIVES the same set from them, through readers that now live in build_index.py beside the schema. The rule's params carries that set's fingerprint, which is the rerun trigger, and the equality of the two sides is what makes the trigger mean anything: a glob over products_dir would merge tiles or exposures from an earlier, larger tile list sharing the root, rows no trigger could see. star_cat_merge depends on a live exposure through its exp_persist manifest and on a RECLAIMED one through its tar, which no rule declares and which therefore requires nothing to be built. Requesting a reclaimed exposure's manifest instead rebuilds its whole chain from VOS, and ancient() does not prevent that: measured on smk-g6 with one reclaimed exposure given a manifest by hand, the dry run grew exp_get_images, exp_split, exp_psf and exp_persist jobs. Reclaimed exposures belong in the star catalogue — carrying their PSF products off scratch is what exp_persist is for. Both rebuild rather than append, so the output is a function of its input set: byte-stable on a no-op rerun (tmp-then-cmp-then-mv), rebuilt when a unit is appended. Neither is a localrule — one job over ~20k units is real work. star_cat_merge produces no job, and a parse-time warning rather than a runtime failure, when persist_exp keeps no validation_psf-shaped file. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
PERSISTENCE KEYED ON THE WRONG FILE, and the consequence was the avalanche it exists to prevent. persist_targets() skipped an exposure only when its SCRATCH tombstone was there, but clean_exposure is one of two ways a store disappears and the other leaves nothing behind: /scratch is purged on a 60-day window whether or not the workflow reclaimed anything. After a purge — or on any campaign with clean: false — every exposure looked live, its persist manifest was requested, its exp_psf manifest was gone, and snakemake rebuilt the whole exposure chain from VOS. The test is now exp_store_reclaimed(), used by persist_manifests() and by star_cat_merge's live/reclaimed split so the two cannot disagree, and it takes BOTH pieces of evidence that an exposure once had a store: a tombstone, or a persisted manifest with no exp_psf manifest beside it. Both are needed. The manifest clause alone regressed the tombstone case — measured on smk-g6, whose 126 exposures were cleaned before exp_persist existed and so have no manifest: the dry run grew 126 exp_get_images, exp_split, exp_psf and exp_persist jobs, a whole campaign rebuilt from VOS. The tombstone clause alone is the purge bug above. Neither file can be dropped, because the parse cannot otherwise tell a store that is GONE from one not BUILT yet, and a fresh campaign must still be asked to persist. OVERLAPPING KEEP PATTERNS FAILED EVERY EXPOSURE. persist_exp treated a file matched by two patterns as a flat-member name collision, so 'validation_psf-*.fits' alongside '*.fits' — an ordinary way to write a keep list — aborted the pack. Two DIFFERENT paths on one member name is still fatal; the same path twice is now one file, recorded under the first pattern that matched it. merge_star_cat MATERIALISED THE WHOLE CAMPAIGN before merging a row: every member's bytes, ~2 MB per exposure, ~40 GB at DR6's ~20k exposures against a rule asking for 16 GB. TarMembers hands the merge class the same entries one tar at a time, so peak memory is one member plus the class's own accumulators, which are the unavoidable term. It keeps __len__ off the manifests so the count is still logged before a tar is opened. THE STAR MERGE RERAN ON BOOKKEEPING. Its fingerprint was over input PATHS, and an exposure's edge flips from its manifest to its tar the moment its store is reclaimed — so every reclamation pass reran the merge over identical content. It is over the exposure IDS now, which move only when the set does, and which are what the job derives on its own side. final_cat_merge's is over tile ids for the same reason. merge_final_cat's MISSING-COLUMN CHECK WAS UNREACHABLE. create_final_cat's read_data wraps its column selection in a bare `except:` that prints and falls through, so a missing column left its return values unbound and the caller got UnboundLocalError from the return statement, naming nothing. The columns are checked against the catalogue's own header before read_data is called, and the message now names every missing one. A TILE LISTED TWICE killed the merge on the second create_dataset. The tile list is appended to by hand, so duplicates happen; campaign_tiles() dedupes it order-preserving, and the Snakefile's TILES does the same so the fingerprint and the job's derived set still name the same set. merge_class OFFERED MCCD AND SETOOLS while only MergeStarCatPSFEX had learned the [fileobj, name] entry shape. Both now take the entry's name from its last element like PSFEX does — unchanged for the module runner, whose entries are [path]. Setools needs one thing more before it can read a tar (it hands file_io input_file_list[0][0] as a template path), and merge_class says so rather than implying otherwise. Also: README no longer lists scripts/sp_rule.py, which does not exist. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
Three defects in scripts/python/create_final_cat.py, fixed where they live
rather than worked around in the workflow rule that now calls it. A hand-run
of the tool deserves them as much as the rule does, and files it has already
written carry the first one.
copy_data allocated np.empty with the SOURCE catalogue's full dtype and then
filled only the requested columns, so every column NOT in the parameter file
reached the hdf5 file as uninitialised memory: meaningless values, and
different bytes on every run over the same inputs. It allocates the requested
columns alone now, in the source catalogue's order. The parameter file says
what the merged catalogue is for; those are the columns it gets.
read_param_file returned list(set(...)), whose order varies with the process's
string hash seed. Column order is part of a structured dtype and therefore
part of the file, so two runs over the same inputs disagreed. Ordered dedup
via dict.fromkeys. (The duplicate-count message also only printed for more
than one duplicate, and said {n} literally.)
read_data wrapped its column selection in a bare `except:` that printed and
fell through, leaving its return values unbound — so a missing column surfaced
to the caller as UnboundLocalError from the return statement, naming neither
the file nor the column. It raises a KeyError naming the file and every
missing column, in parameter-file order.
process()'s own create_dataset follows the array copy_data returns rather than
the source dtype, which are no longer the same thing.
merge_final_cat.py drops the equivalents it had been carrying at the call site
and relies on the fixed functions. The fixture hdf5 is byte-identical either
way (md5 6de2d261…): the workaround and the fix produce the same file, which
is the point.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
Both merges had a constant mem_mb, which is wrong by however much a campaign
differs from the one it was tuned on — and these are the only two rules whose
single job grows with the whole campaign. Both are now measured slopes,
evaluated against the campaign's own bytes at DAG build, still * attempt.
STAR SIDE, measured on this login node in the campaign container over
synthetic tars, 20 and 80 exposures of 40 CCDs x 400 stars:
input members peak RSS (getrusage RUSAGE_CHILDREN)
32.3 MB 383 MB
129.0 MB 1313 MB
a slope of 10.1x input bytes over a ~73 MB interpreter floor. Tenfold because
MergeStarCatPSFEX accumulates every column into python LISTS of python floats
before building the output arrays. THE CONSEQUENCE IS A CEILING and the
Snakefile says so: at ~2 MB of members per exposure a 16 GB job merges roughly
800 exposures, and DR6's ~20k would want ~400 GB. A full-survey full_starcat
needs that accumulation changed to preallocated arrays or a two-pass count —
a change to MergeStarCatPSFEX, not to this rule, and not in this PR. The
formula is honest about the slope so the job asks for what it will use and
fails at submission rather than most of the way through.
TILE SIDE, measured against smk-g6's real catalogues, 2 tiles (73.9 MB in,
largest 39.6 MB) and 6 tiles (235.5 MB in, largest 47.7 MB): peak RSS 129 MB
and 139 MB. FLAT in the tile count, because the merge holds one catalogue at a
time — so it is sized on the LARGEST tile at ~3x, not on the total. Runtime is
the total, since every tile is read end to end. On smk-g6's 64 tiles the rule
resolves to mem_mb=1002, runtime=51, against the flat 8000/120 it had.
Sizes come from stat() on the tar or the catalogue, falling back to the
measured per-unit default when a fresh campaign has not produced it yet.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
Closes the readability half of #844, and makes the 2026-09-08 call's request — keep the PSF model — something you can write down as `psf_model` rather than `*.psf`. `persist_exp:` entries are now names from a catalogue in persist_exp.py, which is the single source of truth for what each one means, what it costs per exposure and what keeping it buys: psf_validation validation_psf-*.fits 2.0 MB psf_model *.psf 2.8 MB psfex_cat psfex_cat-*.cat unmeasured star_selection star_selection-*.fits 24.5 MB star_train star_split_ratio_80-*.fits 19.9 MB star_test star_split_ratio_20-*.fits 7.1 MB star_stats star_stat-*.txt unmeasured `persist_exp.py --list-products` renders it, and config.yaml's block IS that rendering rather than a second copy of it — the old block was a long comment listing globs and their sizes, maintained by hand beside the code that actually knew them. THE DEFAULT BECOMES psf_validation + psf_model, ~4.8 MB per exposure. The model is the single most capability-adding thing an exposure can keep: with it the PSF can be re-interpolated at any position later without rebuilding the chain from VOS, and without it that capability dies with the /scratch purge. A raw glob is still accepted as an escape hatch for a file the catalogue does not name yet. The test is syntactic and cheap — a glob metacharacter or a dot means glob, a bare identifier means name — so `*.psf` and `psf_model` cannot be confused. An unknown NAME is a parse-time WorkflowError listing the valid ones, not a silently empty keep or a per-exposure failure an hour in. The manifest records both: `products` as written, `patterns` resolved, and each member's own `product`. star_cat_merge's gate and its member glob resolve through the same catalogue, so adding a product cannot leave the two disagreeing, and the "add this to persist_exp" hint now names the product. Tile-side retention is explicitly out of scope and noted as such in config.yaml: final_cat is the only tile product that persists today, and it is written straight to products_dir by tile_make_cat. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
All three merge classes built every output column by extending a python list
with one value per star: `x += list(data["X"])`. Four bytes of float32 payload
became a 32-byte python object plus an 8-byte pointer in an overallocating
list, measured end to end at ~10x the input bytes — which put a full-survey
full_starcat (~20k exposures x 40 CCDs) at ~400 GB of RAM and out of reach of
any node.
One array per input catalogue per column, concatenated once at the end. Same
values, same order, same dtypes — np.array() over a list of numpy scalars and
np.concatenate() over the arrays they came from agree on both. The stacking
helper empties the list it is handed, which is half the saving: concatenate
holds the chunks and the result at once, so releasing column by column peaks
at one campaign plus one column rather than two campaigns.
MEASURED on the same two fixture points, 20 and 80 exposures of 40 CCDs x 400
stars:
input members peak RSS, before after
32.3 MB 383 MB 238 MB
129.0 MB 1313 MB 740 MB
10.1x -> 5.5x, over a ~62 MB interpreter floor. The rule's mem_mb factor
follows. A 16 GB job now merges ~2200 exposures rather than ~800.
THE REMAINING 5.5x IS THE OUTPUT SIDE: file_io writes every float column as
FITS 1D, so float32 inputs become a float64 table astropy then buffers. That
is a change to the output FORMAT, which is what sp_validation reads, and a
different decision from this one.
BYTE-IDENTICAL OUTPUT, both ways in. The workflow's tar path and the module
runner's plain [path] path produce the same file as before the change, md5
f7caa1cf… on the fixture — the runner path checked by calling
MergeStarCatPSFEX directly with [[path]] entries as merge_starcat_runner
builds them.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
`persist_exp:` was doing two jobs. It decided what a campaign keeps for later — a retention question, and the user's — and it also decided whether the campaign's star catalogue could be built at all, because star_cat_merge existed only when the keep list happened to name something validation_psf-shaped. That made the survey's PSF diagnostics an opt-in, and a typo away from silently absent. exp_persist now packs psf_validation for every exposure whatever the config says. It is the merged catalogue's PROVENANCE — a full_starcat with no per-exposure inputs beside it cannot be audited, re-cut or recomputed after a purge — and it is what keeps APPENDING TILES CHEAP, since a tile added next month brings exposures whose catalogues must join the existing stack and the alternative is rebuilding their chains from VOS. ~2 MB per exposure: ~40 GB and ~40k inodes at DR6 scale against a ~1 M-inode group quota, which is the price of being able to say where the number came from. `persist_exp:` is therefore purely additive retention, defaulting to psf_model, and an EMPTY list is now a coherent instruction rather than a switch that turns persistence off: the tar holds the merge's inputs and nothing else. The keep-list gate on star_cat_merge and its parse-time warning are gone with it, as is clean_exposure's conditional edge on exp_persist — there is no configuration left under which that rule has nothing to wait for. No transient/cleanup knob: these files are kept, not staged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
…t all final_cat_merge on smk-g6's real catalogues failed on three of the 67 columns the parameter file asks for. Two of them the file should not have been asking for. IMAFLAGS_ISO is DROPPED. The tile-side SExtractor runs with FLAG_IMAGE = False and DOT_PARAM_FILE = default_noimaflags.param (config_tile_Sx.ini), so the column is never written into a tile catalogue and asking for it could only fail. Instrument flags reach the pipeline on the EXPOSURE side, where exp_split delivers the flag image and SExtractor reads it. NGMIX_MOM_FAIL is RENAMED to NGMIX_MCAL_TYPES_FAIL, which is what f0fca23 called it in June and what the catalogues carry. NGMIX_NEIGHBOUR_FLAG STAYS. It was added to make_cat in fa6e001 on 2026-07-12 and it is the blend flag the systematics tests need. smk-g6's catalogues do not have it — checked on the files — so final_cat_merge still fails there, and that failure is correct: the campaign's catalogues are missing a column the analysis wants, which is a fact about the data and not about this file. Its launch snapshot is gone (only .snakemake survives under smk-g6-state), so the run's HEAD cannot be read back; what remains is that its catalogues carry NGMIX_MCAL_TYPES_FAIL (June) but not NGMIX_NEIGHBOUR_FLAG (July), consistent with a snapshot taken between the two. NO MASK COLUMN REPLACES IMAFLAGS_ISO, AND THE FILE NOW SAYS WHY. The intended replacement is make_cat's per-band MASK_<band>, queried from the healsparse maps named by MASK_EXT_PATHS — and the workflow sets none: config_tile_Mc.ini has no such entry, save_mask_ext_data is never called, no MASK_<band> column exists in any catalogue this workflow has produced, and smk-g6's carry none. Naming one here would fail every merge on every campaign. The merged catalogue therefore carries no mask information today; that is a CONFIG gap, and closing it is setting MASK_EXT_PATHS first and adding the column names second. No healsparse map is staged under /project/def-mjhudson yet. With NGMIX_NEIGHBOUR_FLAG set aside, the merge runs clean over all 64 of smk-g6's real catalogues: 2.50 GB read in 20 s at 151 MB peak RSS, producing a 0.94 GB hdf5 of 65 columns. That also confirms the tile-side sizing — the rule asks for 1002 MB and 51 minutes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
The array accumulation landed a commit ago took the star merge from ~10x the
input bytes to ~5.5x. What was left was the accumulation itself: one array per
input catalogue, then a concatenate that has to hold its inputs and its result
at the same time.
MergeStarCatPSFEX now makes two passes. The first reads only the FITS HEADER
of every input — NAXIS2, the row count — and touches no data block; the second
allocates each output column once, at its exact final length, and fills it
slice by slice. There are no chunks and no concatenate, so peak memory is one
output plus one input catalogue.
The workflow's tar reader hands over the archive's own file object rather than
a BytesIO of the whole member, so the counting pass costs a header rather than
a member. Both it and a plain list of paths are iterable twice, which the two
passes require; a one-shot iterable would fill nothing on the second pass, so
the merge checks that the passes agree on the row count rather than writing a
catalogue padded with uninitialised memory.
MEASURED on the same two fixture points, 20 and 80 exposures of 40 CCDs x 400
stars:
input members python lists arrays+concat two passes
32.3 MB 383 MB 238 MB 221 MB
129.0 MB 1313 MB 740 MB 661 MB
slope 10.1x 5.5x 4.8x
The rule's mem_mb factor follows. A 16 GB job now merges ~1300 exposures.
THE REMAINING 4.8x IS THE OUTPUT SIDE: file_io writes every float column as
FITS 1D, so float32 inputs become a float64 table astropy then buffers — 141 MB
of table for 78 MB of payload at the 80-exposure point. What stands between
here and a full-survey full_starcat is that format, not the merge.
MergeStarCatMCCD and MergeStarCatSetools keep the array accumulation. Their
process() computes campaign-wide statistics over the same columns, so a
two-pass rewrite there is a larger change with no consumer today — psfex is
what every campaign runs.
BYTE-IDENTICAL OUTPUT, both ways in: the workflow's tar path and the module
runner's plain [path] path both give md5 f7caa1cf… on the fixture, unchanged
through both rewrites.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
The rule read every tile's catalogue on every run, because a DAG output must be a function of its input set and rebuilding is the simple way to guarantee that. At DR6 scale it is also ~800 GB of IO to add one 35 MB tile. It now brings the file INTO AGREEMENT with the campaign: a tile with no dataset is added, a dataset whose tile has left the campaign is deleted, a dataset whose source catalogue CHANGED is re-read, and one that agrees with its source is left alone, unread. Each dataset records its source's size and mtime as attributes, and a mismatch is what changed means — which is also what keeps the file from drifting from its inputs the way an append-only tool does. create_final_cat.py's own process() implements only the append-only half of this, skipping any tile already present whatever the file on disk now says. WHAT IS AND IS NOT A FUNCTION OF THE INPUT SET, since this is the guarantee being traded. The file's CONTENT is: the same tiles with the same catalogues give the same datasets, the same columns and the same n_tiles, whether they arrived at once or one campaign at a time. Its BYTE LAYOUT is not, because hdf5 lays a group out in the order things were added. That is the price of not re-reading the campaign. UNTOUCHED ON A NO-OP, which is stronger than the byte comparison it replaces and cheaper to establish: reconciling is PLANNED against a read-only open, and an empty plan never opens the file for writing, so its mtime cannot move. A non-empty plan is carried out on a copy which is then moved into place, so a crash mid-merge leaves the old catalogue intact. VERIFIED on a three-tile fixture: build (3 added), no-op (unchanged, mtime identical to the nanosecond), append one tile WHILE AN EXISTING TILE'S CATALOGUE IS UNREADABLE — chmod 000, which succeeds and reports 1 added, so the existing tiles were demonstrably not read — rewrite of one catalogue (1 refreshed), and dropping two tiles from the list (2 removed, datasets gone, n_tiles 1). A from-scratch build of the same set is byte-stable across reruns. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
OPTIONAL COLUMNS WERE DECIDED ONCE FOR THE WHOLE MERGE. MergeStarCatPSFEX read MAG/SNR/ACCEPTED out of the first catalogue's dtype and applied that verdict to every file behind it, so a merge over a mix of ordinary and pix2wcs-converted catalogues was wrong in both directions: ordinary first raised KeyError on the first converted file, and converted first SILENTLY ZEROED the real values of every ordinary file. The dtype now comes from any file that carries the column and pass 2 asks each file for its own schema, so only the files that actually lack a column are zero-filled. Both orderings verified on fixtures. A FAILED psfex_interp COULD GET A GREEN MANIFEST, and clean_exposure takes that manifest as its go-ahead to delete the store. With the default retention list, an exposure whose interpolation failed but whose PSFEx model landed had a non-empty match set, so the pack succeeded and the stars went with the store — unrecoverable short of rebuilding the chain from VOS. psf_validation is not optional: nothing matching it now fails the job, while the store is still on disk. Retention products that match nothing stay warnings. SHRINKING THE KEEP LIST DELETED PRODUCTS FROM /project. The list rides on `params`, so editing it reruns the pack — which rewrote the tar without what had been dropped, on the backed-up filesystem, with the scratch store it came from usually already reclaimed. RETENTION IS NOW ADDITIVE: an existing tar is a FLOOR, its members carried into the new one whatever the current list says, so a config change can only ever add. Removing a product is a deliberate act on products_dir, not a config edit. Verified: pack with [psf_model], rerun with an empty list, the .psf is still there under its own product name and the tar is byte-identical. THE COLUMN SET REACHED NO RERUN TRIGGER. final_cat_merge's reconcile keyed staleness on each source catalogue's size and mtime, and the column set is not a source catalogue: final_cat.param arrives through `params`, and the hash covered workflow/scripts/ only, not scripts/python/create_final_cat.py. So this PR's own edit to final_cat.param would have left every dataset in an existing hdf5 written to the old schema with nothing to notice. The file now carries a digest of the resolved column list on its root and refreshes every tile when it moves, and MERGE_FINAL_HASH covers all three files the rule's behaviour comes from. Verified: build, edit the parameter file, rerun -> 3 refreshed. STAR_CAT_MERGE'S MEMORY WAS SIZED ON THE TAR, which holds whatever the campaign retains, while the merge reads the psf_validation members alone. Measured on a fixture with a 3 MB PSF model kept: the tar is 92x the members it will read, and the default retention is 2.4x. It also jumped discontinuously as exposures were packed. The manifests record the product each member came from — exactly so this is answerable without opening a tar — so the sizing sums those members. NO REQUEST WAS CAPPED. A mem_mb above the partition maximum is a job SLURM never schedules and snakemake never diagnoses: it sits PENDING while the campaign looks alive. Both merge formulas grow with the campaign, so at some size they cross it. `max_mem_mb:` (default 750000, for Nibi's 766 GB standard node) caps both, with a parse-time warning naming the rule that was capped. copy_data ORDERED ITS OUTPUT BY THE SOURCE CATALOGUE, which made the merged dtype a property of the catalogue rather than of the parameter file: the ordered dedup added to read_param_file had no effect, and two tiles written by different ShapePipe versions landed in one group with two different structured dtypes, which np.concatenate refuses. It orders by param_list now. Verified on two catalogues with reversed column orders and an extra column: one dtype, concatenate works. The fixture hdf5 md5 moves with the column order, d2882294… -> 43ff946d…. RECONCILE LEAKED SPACE. It copied the file and deleted datasets in place, and HDF5 never reclaims that, so every refresh of a tile grew the file by that tile. A plan that removes or refreshes anything now builds the tmp fresh, moving the datasets it keeps across with h5py's own group copy — a dataset-level copy that never reads a row into numpy — so the result is compact; pure-append plans still copy and append. Verified: five successive full refreshes leave the file the same size, and dropping a tile shrinks it. Also: comments referring to the deleted parse-time keep-list gate are gone; `-s add` is documented as what it is (accepted by create_final_cat.py's validator, then falling through to the ordinary walk, so not a way to add one tile by hand); and three latent issues are noted where they live rather than fixed — hdu.columns.dtype ignoring TSCAL/TZERO (no validation_psf column is scaled), MergeStarCatSetools rebinding its ellipticity accumulators so only the last file's reach the output (pre-existing, setools is not wired to any workflow path), and the .tmp a SIGKILL can orphan next to the catalogue. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
… the tile one The campaign's two products behaved differently for no reason anyone chose. The shear catalogue reconciled — an append read the appended tiles — while the star catalogue was one flat FITS table that had to be restacked from every exposure the campaign had ever seen to add one: ~40 GB of members at DR6 scale to add ~2 MB, held in memory while it happened. Now they are the same thing. <products_dir>/full_starcat_<campaign>.hdf5, one dataset per exposure at exposures/<exp>, each holding that exposure's every CCD's rows with a CCD_NB column, an n_exposures root attribute and the same column digest the tile side carries. Named for the campaign exactly as the shear catalogue beside it is. CCD_NB IS AN INT: it is parsed out of the member name, where it is always digits, so a string buys nothing and costs 8 bytes a row against 4. DTYPES ARE NATIVE: float32 stays float32, where the FITS writer widened every float column to 1D, doubling both the file and the peak memory of the job that wrote it for no information. MEMORY IS NOW FLAT IN THE CAMPAIGN — one exposure at a time — so the rule is sized on the largest exposure's members rather than the campaign's, and the ~240 GB a DR6-scale flat table would have wanted is simply not a number any more. The Snakefile's sizing block keeps the measurements that got us here, because they are the argument for the format. THE RECONCILE MACHINERY IS NOW ONE MODULE, workflow/scripts/hdf5_reconcile.py, used by both merges rather than duplicated: plan against a read-only open, add/refresh/remove, refresh everything when the column digest moves, compact rewrite when anything is removed or refreshed, untouched on a no-op. Writing it twice would have been two chances to disagree about what an output owes its inputs. THE WORKFLOW NO LONGER CALLS MergeStarCat* AT ALL, so merge_star_cat.py drops the shapepipe import and the psf-model switch, and the [fileobj, name] entry shape those classes learned for it is REVERTED — with no caller it was upstream surface with nothing behind it. What stays upstream is what fixes the module runner's own problems: the two-pass allocation, and asking each file for its own optional columns instead of deciding once for the merge. The runner path is byte-identical to before all of it, md5 f7caa1cf… on the fixture. VERIFIED on the fixtures: build (2 added); no-op (unchanged, mtime identical to the nanosecond); append one exposure with the others' tars at chmod 000, which succeeds and reports 1 added, so they were demonstrably not read; remove one (1 removed, dataset gone, n_exposures 2). Every one of the 16 columns equals the FITS version's values. The tile side's compaction sequence was re-run against a fix this work exposed — the keep-what-changed path called Dataset.copy, which does not exist, and only bites when a plan both rewrites and keeps something. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
path_hash KILLED EVERY INVOCATION over a file one rule needs. It runs at module level, so a snapshot without scripts/python/ raised FileNotFoundError during the parse — taking `sp --unlock`, `sp report` and every dry run with it, and taking them with a bare traceback rather than the diagnosis merge_final_cat.py already carries for exactly this case, which the job could never reach. The hash degrades to a sentinel and warns once; the parse survives, the rule still exists, and the job prints the message written for it. Verified both halves with the file moved aside. THE STAR MERGE'S OPTIONAL COLUMNS HAD NO CANONICAL DTYPE. MAG/SNR/ACCEPTED took the dtype of whichever file carried them, falling back to X's float32 when none did — so an exposure whose files are all pix2wcs-converted got a float32 ACCEPTED while its neighbours got int32, and datasets under exposures/ differed in dtype. np.concatenate refuses that, and no digest can repair it because nothing about the schema CHANGED. The three are pinned (int32, float32, float32) and cast. Verified on two exposures, one carrying them and one not: identical dtypes, concatenate works. A MISLABELED MANIFEST ABORTED THE CAMPAIGN. is_member() accepts a member by product label OR by file name — right for "did this exposure keep the product" — but read_exposure() selects by name alone, so an entry labelled psf_validation whose name did not match put the exposure in the merge and then killed the whole job when the tar held nothing selectable. Membership is now the name on both sides, with one line saying an entry was labelled and skipped. RENAMING `campaign:` WOULD HAVE HALF-UPDATED THE FILE. The tile hdf5 carries the campaign in its GROUP, so a rename pointed the rule at a new group inside the same file: a second group beside the first, the first frozen and stale, and n_tiles describing one of them. One file is one campaign — apply refuses and names what is already there. APPEND IS CHEAP IN READS, NOT IN WRITES, and the docstrings said otherwise. The existing file is copied so the result can be moved into place atomically: one pass over it and, briefly, twice its size on disk. Corrected, and apply now refuses when the filesystem cannot hold it rather than filling /project and leaving a truncated tmp beside a catalogue people trust. A CORRUPT TAR RAISED A RAW ReadError, on both sides. persist_exp now refuses to write a new tar and says the old one is untouched and may hold products nothing else has; merge_star_cat names the tar and says not to delete it. THE 16-COLUMN SCHEMA IS DEFINED TWICE and nothing held the two together. MergeStarCatPSFEX writes the flat FITS table the module runner emits; merge_star_cat.py writes the hdf5. Separate implementations are right — only one of them reads tars, keeps native dtypes and reconciles — but a column added to one writer would simply be missing from the other's product, found by whoever next computed rho statistics from the wrong one. tests/unit/test_star_cat_columns.py asserts the names and their order agree; verified passing, and verified failing when one list is changed. Also noted where it lives: adding a retention product re-packs the tar and moves its mtime, so the star merge refreshes those exposures although their validation members are byte-for-byte unchanged — seconds per exposure against per-member bookkeeping on every exposure, which is not a trade worth making. Stale docs updated to the hdf5 product: the Snakefile's merges header, the README's star_cat_merge paragraph and scripts list (the hdf5 paragraph written last round never landed — its edit script aborted before writing), and config.yaml's psf_validation block. The README now says there are two writers and names the test that keeps their schema together. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
Two rules carry state across invocations and are correct only over SEQUENCES: hdf5_reconcile brings a catalogue into agreement with a campaign that changes under it, and persist_exp packs a tar whose existing members are a floor. Neither is a claim the example tests can finish making, so each gets a hypothesis state machine that walks a random sequence of campaign edits and asserts the model after every one. hdf5_reconcile: units added, refreshed and removed, and the column set flipped, against a real file. After every step the datasets are the campaign's units with their sources' content, the count and digest attributes agree, every dataset shares one dtype, and a no-op leaves the mtime alone. Source mtimes are set explicitly, so a same-size rewrite inside one filesystem tick cannot masquerade as a refresh. Compaction is asserted where the module actually claims it — the rebuild path — and stated as "does not grow with history": a rebuild costs ~1.4 kB more than a from-scratch build (h5py's group copy writes more metadata than create_dataset does) and that overhead is constant, which test_repeated_refresh_does_not_grow_the_file pins directly. Plus a crash injected at the rename, which must leave the previous file byte-identical. persist_exp: random keep lists of product names, raw globs and overlapping mixtures over a store that gains and loses products. Members are additive across packs, never duplicated, and the manifest agrees with the tar down to the product labels; a missing psf_validation fails without writing a manifest, an unknown product name is refused before any work, a corrupt tar is left exactly as it is, and two different sources with one member name are still fatal. Both files were checked against five mutants (never rebuild, never refresh, non-additive retention, optional psf_validation, tolerated collision); each is caught. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
ReconcileMachine.change_columns rewrites every source as it flips the column set, so the stamps alone refresh every unit and disabling schema invalidation survives the state machine. The new test holds every stamp fixed and changes only the requested columns, so the digest is the only thing that can refresh a unit. With the digest comparison in plan() forced off, this test fails and the machine still passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The index keeps a tile's tile_exposures edges after its exposure list goes missing, because clean_exposure's consumer sets must still see it. Readiness was read off those same edges, so a tile named in missing.json stayed ready for the merges (campaign_tiles) and for the Snakefile's TILES_READY. A build now deletes a missing tile's tiles row and keeps its edges. ready_tiles() reads the tiles table alone, which is cheap enough for every job's parse, and both campaign_tiles and TILES_READY go through it. The consumer sets still read the edges. Test: tests/unit/test_build_index_readiness.py (the probe's missing tile is not ready, its edge survives, and it returns when its list does). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A rewrite killed between writing its tmp and renaming it leaves the tmp beside the catalogue. The next run deleted it only after check_free_space, so the orphan's bytes counted against the margin: 2.5x the file free became 1.5x, below the 2.1x check, and the retry refused itself. The single writer per output owns that tmp, so it is now removed first. Test: test_hdf5_reconcile_props, with injected disk accounting as in the review probe. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An add-only merge copies the existing file, attributes included, and then stamped only the provenance keys the new record carried. A merge without a snapshot wrote code_head=unknown beside the previous snapshot's branch, dirty flag, dirty files and time. Every code_* attribute is now cleared before the new record is stamped. Test: test_hdf5_reconcile_props (a full snapshot, then an add with head=unknown, leaves code_head alone). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… manifest clean_exposure named the exp_persist manifest for every exposure. For a reclaimed one, a persist_exp: edit changes exp_persist's params, so the manifest reruns, and it sits behind exp_psf's manifest, which went with the store: snakemake scheduled exp_get_images, exp_split, exp_psf, exp_persist and clean_exposure again for every reclaimed exposure. The edge now takes the leaf treatment the defect edge uses on #887: a live store is asked for the manifest, a reclaimed one for its tar if it exists (no rule's output, so a leaf), else nothing. One lambda per edge is kept. CONTRACTS' persist edge says so. Test: the review's retention dry run (a completed, reclaimed two-exposure fixture with recorded metadata, then persist_exp: [psf_model, star_stats]) scheduled 13 jobs including the full exposure chain for both exposures; it now schedules final_cat_merge, star_cat_merge and all (3). Harness: /automnt/n17data/cdaley/scratch/smoke-879-mine/retention/test_retention.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The prefix match took nibi's login node (c6.nibi.sharcnet) for candide, so the two candide-data cluster tests errored there. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Resolve disposable campaigns through SnakemakeApi so workflow contracts can inspect actual jobs rather than reconstructed source snippets. Seed campaign history with build_index and let the compute parse index two ready tiles, including shared and out-of-scope exposure consumers. Keep state, image-resolution sentinels, and source caches under tmp_path; restore imports, environment, cwd, and Snakemake globals between parses. The existing tests discovery root includes this tier. Co-Authored-By: GPT-6 Astra <noreply@openai.com>
Inspect resolved job inputs for data+psfex, data+mccd, and image_sims+fake. Reclamation must wait on PSF persistence exactly when PSF products exist, and on every in-scope vignets consumer. Campaign merges must read every ready tile and keep product paths and labels tied to products_dir and run:. Missing run: must fail during parsing even with literal paths. Keep final_cat_merge for image simulations, as the workflow explicitly specifies; only exposure persistence and the star merge depend on a fitted PSF. Mutation probes cover gates, missing and out-of-scope edges, renamed products, scratch routing, directory-derived campaign names, and removal of the required run key. Co-Authored-By: GPT-6 Astra <noreply@openai.com>
Changing unit_pre or a rule's params.pre can rerun finished tiles against reclaimed exposure stores. Pin the normalized data+psfex prologues and rendered shells so that change requires explicit review and --update-params-pin at a fresh campaign boundary. Keep the rules' full thread counts when rendering, and require params in both profiles' rerun triggers. Hash every stage and declared rule, including all resolved wildcard instances, and retain normalized payloads in the temporary fixture for diagnosis. Check path independence across distinct roots and document API access, scope, regeneration, and mutation probes. Workflow contracts now name their enforcing checks. Co-Authored-By: GPT-6 Astra <noreply@openai.com>
psf_model=mccd is refused before a DAG exists, so it leaves the resolved modes and gets a refusal test; the required-run: message names unexpanded variables too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cailmdaley
added a commit
that referenced
this pull request
Sep 26, 2026
…strument-defect-map-v2 clean_exposure keeps one lambda per edge: #879's persist edge (tar as a leaf once reclaimed) and the defect edge.
cailmdaley
added a commit
that referenced
this pull request
Sep 26, 2026
…eat/retire-versioning-v2 clean_exposure keeps one lambda per edge: the persist edge from #879 (tar as a leaf once reclaimed) and the footprint edge through footprint_edge().
cailmdaley
marked this pull request as draft
September 28, 2026 12:44
cailmdaley
added a commit
that referenced
this pull request
Sep 28, 2026
* workflow: image simulations as an input mode of the real-data workflow Image-sim m-bias ran through the legacy bash runner and example/cfis_image_sims, a config chain frozen at an older pipeline state (no background vignets, no bkg-rms ngmix, no neighbour/mom-fail flags; its default.* symlinks dangle since e32c4fb). An m measured that way does not calibrate the pipeline that makes the real catalogue. This makes the simulations an input of the same workflow: - input_type: data | image_sims selects $SP_CONFIG. config/cfis_image_sims is an overlay: real files only for the ingestion INIs whose input naming differs (tile Git/Uz/Fe, exposure Gie/Sp -- the sim get_images writes the real-data output names, so every later stage reads identical patterns), everything else symlinked into config/cfis. - psf_model: fake -- the simulations' true PSF. The exposure stage runs only SExtractor (background maps for the vignets); tile_vignets runs fake_interp_runner, which writes galaxy_psf from `psf_dict` under the name the shared configs read (${SP_PSF}_interp_runner). Star-bearing sims can run psfex/mccd unchanged. - unit_pre writes dashed tile numbers for sims (get_images substitutes them verbatim) and exports PSF_DICT when set. A data run's prologue is byte-identical (checked by diffing dry-run shell commands against the base), so no finished data unit is rerun by the params trigger. - bin/sp: SP_PROFILE selects profiles/<name>; SP_RUN_CONFIG replaces (not layers on) workflow/config.yaml and is snapshotted with the code; the venv is optional when snakemake is on PATH. container.py follows both. - profiles/candide: SLURM profile for candide (node-local /tmp bound as /local/scratch for the tile store). Note: completeness.py gains the `fake` tables, and its content hash is a param on every rule, so this lands at a campaign boundary like any completeness edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KNJYtm9z6VeoChxtDLYn4W * workflow: image-sims merge column list, and the merge reads the workflow layout create_final_cat.py -I globbed run_sp_tile_Mc_*, the legacy bash runner's dated run dir; the workflow writes an undated run_sp_tile_Mc. Accept both. The overlay's final_cat.param was a symlink to the real-data list, which names columns make_cat does not write (IMAFLAGS_ISO: the tile SExtractor has no flag image; NGMIX_MOM_FAIL: written as NGMIX_MCAL_TYPES_FAIL) and omits NUMBER and NGMIX_MCAL_TYPES_FAIL, which sp_validation's image-sim extract reads. It is now a real file: the legacy example/cfis_image_sims list plus NGMIX_NEIGHBOUR_FLAG, checked column by column against a smoke-run final_cat. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSjZrhgsGCLAJCLb5Zjmc3 * workflow: campaign-unique node-local tile store for image sims; candide excludes The node-local tile store was /local/scratch/sp-<tile>, unique within a campaign but not across concurrent ones. The image-simulation shear branches are concurrent campaigns over the same tile IDs, and on candide /local/scratch is the node's shared /tmp: two branches' tile_shape jobs on n09 shared one store, one tile_vignets wiped it under the other's ngmix, and both failed. The failure can also be silent -- ngmix reading the other branch's vignets. image_sims now prefixes the store name with a hash of the run dir; data campaigns keep the bare name, so their shell commands (a rerun trigger) are unchanged. The tile_shape group's own slurm_extra (the --tmp floor) replaced the profile default and dropped candide's node excludes; its jobs ran on n09 and n17. profiles/candide restates both for the four members via set-resources (slurm_extra only; the rules keep their threads and attempt-scaled mem_mb, checked in a dry run). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSjZrhgsGCLAJCLb5Zjmc3 * workflow: port the MCCD exposure chain to the workflow grammar config_exp_mccd.ini was never ported: it read split_exp and mask_runner outputs through INPUT_MODULE (mask_runner is gone since #847, and with it the pipeline_flag files), had no mask_query, chained setools and MCCD through last:, and left RUN_DATETIME at its default, so the run dir was dated and nothing downstream could find it. psf_model: mccd could not run. It is now the PSFEx chain up to the star selection -- same split-CCD inputs, flag image, mask_query and setools, so both models fit the same stars -- followed by mccd_preprocessing (all CCDs of the exposure into one training and one test catalogue) and mccd_fit_val (fitted_model-<exp>.npy, which the tiles' mccd_interp reads, plus validation_psf-<exp>.fits). merge_starcat and mccd_plots are dropped: they are campaign-level diagnostics, not per-exposure products. The completeness table's MCCD branch had preprocessing at 80 (per CCD; it is 2 per exposure), listed merge_starcat and the plots, and was all warning-only. It now carries the counts this chain produces, mandatory as for PSFEx: an exposure without a model has no PSF on any tile it overlaps. Checked on SKiLLS star sim 1z2z (exposure 2086792): 120/40/80/2 through preprocessing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VSU9pPZjpBjzWorAL88aKQ * workflow: exp_psf reserves 2 cores, not 8, for the MCCD chain MCCD fits one focal-plane model per exposure after the per-CCD stages: 85 min single-threaded for ~2500 stars and 8.3 GB (SKiLLS star sim 1z2z_1, exposure 2086792), and 48 min on 8 BLAS threads -- 1.75x for 8x the cores, which the thread caps forbid anyway. With threads: 8 the fit leaves 7 reserved cores idle for its whole length. The mccd chain now takes 2: the per-CCD stages run 2 wide (minutes), the fit is unchanged, and an exposure reserves about a quarter of the core-hours. PSFEx and fake keep 8. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VSU9pPZjpBjzWorAL88aKQ * mccd_interp: test for N_EPOCH among the column names Multi-epoch MCCD interpolation checked `"N_EPOCH" not in cat.get_data()`, a membership test against the FITS record array, which current numpy rejects ("Cannot compare structured or void to non-void arrays"): every tile failed in tile_vignets with psf_model: mccd. psfex_interp already tests `.dtype.names`; so does this now. With it, tile 233.293 of SKiLLS star sim 1z2z_1 interpolates the seven exposures' MCCD models (a few sparse CCDs lose an epoch, as intended). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VSU9pPZjpBjzWorAL88aKQ * workflow: MCCD validated through the full chain; drop the warn-only flags With the ported exposure chain, the mccd fixes and the mccd_interp fix, psf_model: mccd ran SKiLLS star sim 1z2z_1 tile 233.293 end to end: seven exposures' focal-plane models (120/40/80/2/2 each), mccd_interp's galaxy_psf store, vignets, ngmix and a final_cat of 21857 objects. tile_vignets' mccd counts are now mandatory as for psfex, and the README no longer calls mccd "wired but unvalidated". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VSU9pPZjpBjzWorAL88aKQ * workflow: tile_store_root run-config key moves the tile store's bind, per campaign bin/sp rewrites the source of the `/local/scratch` bind in the launch snapshot's profile (appending one when the profile has none) and creates the directory. tile_local() is untouched: its output is a params rerun trigger, a bind is not, so the key can be set on a resume. candide's 31 GB node /tmp cannot hold dense image-sim tile stores; image-sim campaigns there point this at NFS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qv5MwRiVKa8wnH665dWBia * completeness.py: atomic write for stage log/manifest JSON write_if_changed() used Path.write_text(), which truncates before writing. run_report.py reads every logs/*.json right after compute finishes (the onsuccess hook) and can catch one mid-write as an empty file -- observed during the grid_3 image-sims acceptance test (exp_get_images.json for one exposure came back "Expecting value: line 1 column 1"). Write to a same-directory temp file and os.replace() it into place instead, which is atomic and closes the race. * workflow/config.yaml: universal template, no cluster-specific paths The committed default carried one person's real nibi paths (tile_list, inputs, container, outputs) baked in. That means `sp run` was not actually the same command on every machine: anyone who forgot to set SP_RUN_CONFIG on a different cluster would either hit a confusing filesystem error somewhere downstream, or silently point at paths that happen to also resolve on their machine but belong to someone else's campaign. config.yaml now carries a REPLACE_ME sentinel for every machine- or campaign-specific path, with a top-of-file explanation that a real run always supplies its own via SP_RUN_CONFIG. The Snakefile checks the keys that have no safe fallback (tile_list, inputs.tiles/exposures, outputs.run_dir/index_db) right after the config file is read and raises one clear, identical error on any cluster if one is still the placeholder, rather than failing confusingly later or not at all. `container:` is left out of that check: a local sandbox or cached SIF is resolved before it, so a placeholder there is often harmless. Also cleans up several stale comments left over from earlier campaign states (smk-g4/g5 narrative, `clean`/`clean_tiles` comments describing "OFF" for a value that had already been changed to `true`) and two "knob" -> "setting" wording fixes. * workflow: retrieve mode (symlink|vos) is a run-config key, not fixed in the ini RETRIEVE=symlink was hard-coded in config_tile_Git.ini and config_exp_Gie.ini (both config/cfis and config/cfis_image_sims), which assumes a pre-staged local mirror. That's nibi's layout; candide's real-data ingestion fetches tiles/exposures from VOSpace (RETRIEVE=vos) and has no local mirror. Add `retrieve: symlink|vos` to the run config, validated at parse time, exported as SP_RETRIEVE by unit_pre() and interpolated into the four ini files. input_type=image_sims always forces symlink regardless of the run config's value, since simulation output is generated locally and never lives in VOSpace -- so a `data` run config's `retrieve: vos` reused as a sims campaign's base can't leak into the sims ingestion stages. Verified with a dry run against both input_type=data (retrieve: vos) and the existing image_sims acceptance-test config (still forces symlink) that SP_RETRIEVE renders correctly in each case. * config.yaml: restore real nibi defaults, keep the placeholder check as a guard REPLACE_ME sentinels made the committed config unrunnable as-is on any cluster, including nibi -- its own actual mainline user. Restore the real, working nibi values (tile_list, inputs, container, outputs) so `sp run` with no other setup does something real and correct there, the same way it always used to. The REPLACE_ME check in the Snakefile stays: it now guards against a future edit that strips these defaults without supplying real ones, rather than gatekeeping every run. Running the committed default from candide now fails honestly on nibi's real, unreachable /project path (FileNotFoundError) rather than on the placeholder check -- correct, since candide cannot see nibi's filesystem and must supply its own paths via SP_RUN_CONFIG regardless. * Snakefile: check machine: against SP_PROFILE at parse time machine: (the run config) and SP_PROFILE (the env var bin/sp reads to pick profiles/<name>/, default nibi) are two independent switches for the same fact -- which cluster this is -- set in two different places at two different times, with nothing stopping them from disagreeing: export SP_PROFILE=candide but leave a copied run config's `machine: nibi` unedited, and jobs run under candide's SLURM settings against nibi's data paths. Raise a clear WorkflowError at parse time if they don't match, skipped only when `machine:` is genuinely absent (a config predating this key). No code default for `machine:` -- guessing one on its behalf would defeat the point of the check. Also fixes a comment above the REPLACE_ME check left stale by the previous commit: it still called config.yaml "a universal template with no real paths in it" after nibi's real defaults were restored. * config.yaml: machines: table drives per-machine (and per-input_type) defaults `machine: nibi` selects config.yaml's own real defaults; the same key now also indexes a `machines:` table carrying tile_list, retrieve, inputs, outputs and container for both nibi and candide, so switching `machine:` (kept consistent with SP_PROFILE by the existing check) is enough to point a bare `sp run` at the right cluster's paths. On candide, where the real survey (VOSpace, RETRIEVE=vos) and the SKiLLS simulations (local disk, RETRIEVE=symlink, a different container) need different values, a machine entry nests them under `data:`/ `image_sims:` instead of stating them flat. `$base_dir` in a string is substituted with that machine's own `base_dir:`, so nibi's block states its one common root once instead of four times. A value may be the sentinel `TBD` -- known to be needed, not yet known what it is (candide's real-data ingestion: no local mirror exists there, the real vos: paths and output roots aren't confirmed yet) -- checked at parse time with the same one-clear-error mechanism REPLACE_ME used to provide. Container resolution needed a specific bridge: container.py reads `container:` by regex off the run config FILE, independent of the Snakefile's `config` dict, so a machine-derived container (which only exists after this Python-side merge) would otherwise be invisible to it. Reused SP_CONTAINER (already the documented way to point resolve_image() at a specific image) instead of teaching container.py about `machines:` too -- set only when neither an explicit top-level `container:` nor a user-set SP_CONTAINER already exists, preserving the same override precedence as every other machine default. Also fixes SP_RUN_CONFIG to LAYER on top of config.yaml rather than replacing it wholesale (workflow.configfile(), called conditionally -- it's the plain method the `configfile:` directive itself compiles to, so it works inside an `if`; merges recursively per snakemake.utils.update_config). Necessary for `machines:` to reach the common case at all: every image-sims run and any one-off cluster override already goes through SP_RUN_CONFIG, and under the old wholesale-replace behaviour none of them would ever have seen the table. Safe against the original replace-wholesale concern (an external config silently inheriting an unrelated real campaign's paths) because `machines:` entries are well-scoped defaults gated on `machine:`/`input_type:` and independently checked against SP_PROFILE, not one campaign's actual values reused as another's fallback. Verified: the committed nibi default still fails honestly (on nibi's real, candide-unreachable path) rather than on a placeholder; a machine:/SP_PROFILE mismatch still raises before that; a candide+data run config (all TBD) raises the new clear placeholder error; and the grid_3 image-sims acceptance-test config, with only `machine: candide` added and its explicit retrieve:/container: removed, dry-runs identically to before except for the retrieve: addition already committed separately -- confirming the machines: table now supplies what that config used to have to state itself, without disturbing the already-completed run's resume state. * workflow: one run-config resolver for Snakefile, bin/sp and container.py The machines: table was only understood by the Snakefile. bin/sp read outputs.run_dir/index_db and tile_store_root straight from the config file, so the committed config (outputs now under machines.nibi) broke `sp run` on nibi with a KeyError, and container.py only saw a top-level container: line. scripts/run_config.py now does the whole resolution (config.yaml, SP_RUN_CONFIG merged on top, then machines[machine] [input_type] for anything unset) and all three use it. - container.resolve_image() takes the resolved container explicitly; drops the SP_CONTAINER workaround, which had let a machine default outrank a user's cached SIF, against the documented order. - nibi's machine entry is nested under data: like candide's, so a nibi image_sims run cannot inherit real-data paths. - Unset required keys are reported like TBD, in one error. - README gains a "Run configuration" section; config.yaml, Snakefile and the ini comments are cut down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NzWDtdQbsK1VhvYbDnmTo * run_config: `run:` name, expanded as $run in run-config paths config.yaml gains `run: smk-g6`, and nibi's paths use $run instead of the literal campaign name. $base_dir and $run now expand in values set in the run config itself too, not only in machine defaults, so a sims run config can name its output directories by `run:`. A required path left holding an unexpanded $variable is reported like TBD. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NzWDtdQbsK1VhvYbDnmTo * added user run config template * bin/sp: -c/--config-file for the run config, instead of SP_RUN_CONFIG The run config was only settable through an exported variable: invisible in the command, absent from shell history and job scripts, and silently inherited by later commands. `sp <verb> -c my_run.yaml` now takes it on any verb; sp consumes the flag, makes the path absolute and exports SP_RUN_CONFIG itself, which is what the Snakefile, container.py and the snapshot read. An already-set SP_RUN_CONFIG is still honoured, and -c wins. -c is sp's own flag, not snakemake's --cores; cores pass through as --cores/-j. Closes the `sp run --config-file` half of #891 step 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NzWDtdQbsK1VhvYbDnmTo * bin/sp: document -c, the two-file merge and the environment in the header Typical invocations, the order the run config and config.yaml are read in (and that the machines: table fills the rest), the run_config.py one-liner for inspecting a resolved value, why -c is not snakemake's --cores, and the remaining SP_* variables including SP_PHASE, which sp sets itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NzWDtdQbsK1VhvYbDnmTo * improved commeents * workflow smoothed; running until hdf5 file * bin/sp: don't swallow -c/--config meant for the delegated command The argv scan for sp's own -c/--config-file took every -c/--config/ --config-file anywhere in argv as the run config, including ones that belong to the command sp delegates to. This broke the README's own `sp container exec python -c ...` (container.py already reads SP_RUN_CONFIG, so its own -c is never sp's run config) and `sp run --config clean=false` (the override flag() in the Snakefile is written for). Stop the scan at `--` and at the command after `container exec`, and drop the `--config` alias so snakemake's own `--config k=v` passes through untouched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * workflow/README: add products_dir to the image-sims run-config example Without products_dir, merge_final_cats falls back to run_dir and create_final_cat.py finds no matching tile directory under it, so the rule writes an empty hdf5 and exits 0 -- a failure that only surfaces later, in sp_validation, as "No data found". Giving the example the template's <sim>/run + <sim>/product layout avoids that trap; hardening the rule itself is left to #879, which replaces merge_final_cats for sims. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * run_template.yaml: make it resolve Three problems kept this from resolving: run_config.py only expands $base_dir and $run, so the template's own $my_base_dir and $sim stayed literal and the Snakefile rejected the paths; with no input_type, the run took the data branch (cfis configs, psfex, VOS retrieval) instead of image_sims; and outputs.index_db, a required key, was missing. shapepipe_repo: is also dropped -- nothing reads it. Puts the sim name in run: and uses $run throughout, which run_config.py already expands; the remaining <user-defined directory> placeholders are for hand-editing, same as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * more variable expansion in config file --------- Co-authored-by: martinkilbinger <martinkilbinger@cea.fr> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cail Daley <cail.daley@cea.fr>
martinkilbinger
approved these changes
Sep 28, 2026
Compose N_EPOCH_SLOTS with develop's fourth-moment PSF columns (#859): the fixed slot count applies to every per-epoch family, including HSM_M4_1/M4_2/RHO4_PSF_n. TILE_LIST doc entry dropped as on develop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Develop carries #894 (squashed, with Martin's run_config fixpoint / ${name} expansion), #918's MCCD exposure chain, and #859's fourth-moment PSF columns. Resolution: - run_config: develop's variable table (every top-level scalar, ${name}, fixpoint, `run` written back) with this branch's required `run:` and recursive unexpanded-$var refusal. - MCCD chain (config_exp_mccd.ini, completeness.py, exposure.smk comment): develop's #918 wiring; psf_model=mccd stays refused at parse time for persistence. - final_cat_merge replaces develop's merge_final_cats rule; config.yaml keeps `run:` unset (required). - #859 composes with the campaign products: MergeStarCatPSFEX's two-pass _COLUMNS and merge_star_cat.py's COLUMNS gain the six M4/RHO4 columns (22 in both), and cfis/final_cat.param lists the per-epoch HSM_M4_1/M4_2/RHO4_PSF_n slots. - make_cat: taken from the updated feat/make-cat-fixed-epoch-slots. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oducts Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test_hsm_column_seams reads MergeStarCatPSFEX's read set from its
_COLUMNS table (the two-pass merge reads by table, not by literal);
test_run_config pins ${name} shorthands, their nesting, run written back,
and a run left holding $var reported.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cailmdaley
changed the base branch from
feat/workflow-image-sims
to
develop
September 28, 2026 13:59
Develop dropped snakemake from the image (10f9c53: it is a host tool that wraps each job in the container). The root conftest leaves tests/workflow out of collection where snakemake is absent, and deploy-image.yml runs that directory as its own step after installing snakemake>=9,<10 into a throwaway container, so the image stays snakemake-free and the DAG checks still gate CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Takes develop's decision record (#875 as merged) over the pre-squash copy this branch carried, and drops that copy's anchor test (tests/unit/test_astra_anchors.py, tests/helpers/astra_record.py), superseded by tests/unit/test_decisions.py. detection.detection_source_mode: final_cat.param no longer requests IMAFLAGS_ISO, so the [LINT] (issue #912) is resolved in the rationale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cailmdaley
added a commit
that referenced
this pull request
Sep 28, 2026
…g ngmix's own bits (#854) * ngmix: record silent metacal failures in mcal_flags; fail loudly at 100% mcal_flags (NGMIX_MCAL_FLAGS) faithfully recorded failures ngmix *reported*, but defaulted an absent metacal-type result or missing 'flags' key to 0 -- success -- so a fitter failing silently (sentinel outputs, flags never set) produced a fully-populated catalogue with an all-zero flag column. Close the hole three ways: * absent result / absent 'flags' now contributes FLAG_NO_RESULT (2^30) to mcal_flags and to mcal_types_fail instead of counting as success; * a nominally clean fit (mcal_flags == 0) whose noshear shear is non-finite is flagged FLAG_NO_RESULT -- that contradiction is always a silent failure; * end of loop: RuntimeError when objects were processed but none fitted, and a w_log.error line when 100% of fitted objects carry nonzero mcal_flags. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019dA9sas4vvNBcNKRmi973L * ngmix: log instead of raising on 0-fitted (don't crash a campaign on one empty edge tile) The end-of-run 0-fitted guard from #854 raised RuntimeError, which would abort an entire multi-tile campaign job over a single tile with no valid objects. Demote to the same w_log.error used for the 100%-flagged case: loud enough to catch in review, not fatal to the run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * make_cat: never-fit objects get FLAG_NO_RESULT, not MCAL_FLAGS == 0 _save_ngmix_data pre-fills every NGMIX_* column with sentinels and only overwrites objects present in the ngmix output. An object ngmix never fit (0 epoch to process, or an exception caught and skipped) was absent from that output and so kept the pre-fill default of MCAL_FLAGS = 0 -- read by sp_validation as "fit succeeded" (shapepipe#889). Default NGMIX_MCAL_FLAGS and the per-shear-type NGMIX_FLAGS_<SHEAR> to FLAG_NO_RESULT (imported from ngmix_package.ngmix, not duplicated), the same bit an absent per-type result gets in ngmix.get_mcal_flags. Default NGMIX_MCAL_TYPES_FAIL to len(METACAL_TYPES) (all types failed) rather than 0 failures, since sp_validation's classification_galaxy_ngmix also cuts on MCAL_TYPES_FAIL == 0. NGMIX_N_EPOCH is left at its existing 0 default, which is already the correct "never fit" value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ngmix: extract log_run_health for direct unit testing process()'s end-of-run 0-fitted / 100%-flagged checks were inline and only reachable through the full per-tile pipeline (Tile_cat, vignet catalogues, ...), with no unit coverage. Pull them into a module-level function taking just (w_log, count, n_fitted, n_flagged) so the 0-fitted-logs-not-raises contract can be tested directly. No behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * tests: cover FLAG_NO_RESULT absence handling and 0-fitted logging - get_mcal_flags: an entirely absent metacal type, a result present but missing 'flags', and the mix of an absent type alongside a real failure all set FLAG_NO_RESULT rather than silently reading as success (shapepipe#853). - log_run_health: 0-fitted logs (not raises), 100%-flagged logs, and a healthy run logs nothing (shapepipe#854). - make_cat: an object absent from the ngmix output gets FLAG_NO_RESULT for MCAL_FLAGS and every FLAGS_<SHEAR> column, and len(METACAL_TYPES) for MCAL_TYPES_FAIL, while a present/clean object still reads 0 (shapepipe#889). - test_psf_averaging_properties.py's _SENTINELS contract updated to match (it previously pinned the pre-fix 0-default as the expected absent-object sentinel); its measured-row placeholder for mcal_types_fail moved off 5 now that 5 (== len(METACAL_TYPES)) is the absent sentinel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ngmix: derive every metacal flag column from one per-type flag get_type_flags is the single definition of a metacal type's success: its fit ran, reported flags == 0 and returned a finite shear. An absent result, a missing flags key, or flags == 0 with non-finite or missing g all give FLAG_NO_RESULT. compile_results now derives FLAGS_<SHEAR>, MCAL_FLAGS (OR) and MCAL_TYPES_FAIL (count) from it, so the three columns agree by construction. This extends the non-finite guard from noshear to all five types (a NaN g in 1p/1m/2p/2m also poisons the response), counts such types in MCAL_TYPES_FAIL, tolerates an absent type instead of raising KeyError at save time, and drops compile_results' own .get("mcal_flags", 0) absent-as-clean default. make_cat's never-fit defaults are what ngmix derives for an empty result (get_*({})), so the two sides cannot drift apart. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ngmix, make_cat: @sc contracts for the mcal flag invariant - mcal-flags-zero-means-measured (get_type_flags): 0 means the fit ran, succeeded and returned a finite shear; every NGMIX flag column derives from this, and sp_validation's galaxy cut trusts it. - never-fit-is-not-clean (_save_ngmix_data): rows ngmix never fit take the empty-result flags, never 0; couples to sp_validation's MCAL_FLAGS == 0 && MCAL_TYPES_FAIL == 0 cut. - run-health-logs-not-raises (log_run_health): wholesale failure is logged, never raised, so one tile cannot abort a campaign. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * tests: consumer-side invariant for the mcal flag columns; prune restatements Add test_galaxy_cut_admits_only_measured_objects (contracts mcal-flags-zero-means-measured, never-fit-is-not-clean): synthetic metacal results covering every way a fit can fail to be a measurement, plus never-fit objects, go through compile_results/save_results and _save_ngmix_data; sp_validation's MCAL_FLAGS == 0 & MCAL_TYPES_FAIL == 0 cut must admit exactly the clean objects, each with N_EPOCH > 0 and a finite, non-sentinel shape in all five metacal types, and MCAL_FLAGS / MCAL_TYPES_FAIL must equal the OR / count of FLAGS_<SHEAR> on every row. Mutation-checked: it fails for each injected regression in get_type_flags, the aggregates, compile_results and make_cat's defaults. It subsumes test_get_mcal_flags_flags_absent_result_and_missing_flags_key (deleted) and the flag assertions added to the absent-objects test (reverted; test_absent_obj_id_keeps_sentinel_fill also pins them). The three log_run_health tests become one table over its failure modes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * tests: an empty tile survives Ngmix.process (run-health-logs-not-raises) Drive process() on a tile whose every object has no stamps: it must log the 0-fitted error and still write an empty catalogue. Catches the health check not being called and a raise re-added in process(), which the log_run_health table alone cannot see. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ngmix: 0-fitted log names the expected cause (an empty tile) first Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ngmix: fail fast on a wcs-centroid tile with no propagated OFFSET centroid_source="wcs" reads the coadd-centroid OFFSET the stamp extractor writes into every vignette epoch entry. Vignettes cut before that extractor carry none, and until now that meant make_ngmix_observation raised for every object in turn while Ngmix.process's per-object exception handling quietly turned the whole tile into an empty catalogue. check_wcs_centroid_offset inspects the first object with epochs once, up front, and raises with the fix (re-extract with the current vignetmaker, or set centroid_source='hsm') instead of letting the run masquerade as healthy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ngmix, ngmix_runner: a truly empty tile writes the empty catalogue too Move the all-empty-store guard from ngmix_runner's SqliteDict pre-scan into Ngmix.process itself: a tile whose galaxy entries are all 'empty', or whose PSF entries are all {}, now takes the same per-object skip path as any other unfittable object, reaches log_run_health, and writes the five-HDU empty catalogue campaign completeness checks expect. The runner no longer returns early (and no longer needs sqlitedict directly). Adds an end-to-end test through the shipped shapepipe_run launcher, covering both the empty-galaxy-store and empty-PSF-store cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * tests: close the g2-only and disconnected-flagged-counter coverage gaps test_galaxy_cut_rejects_each_nonfinite_shear_component parametrizes the existing non-finite-shear invariant over both g components (not just g1/mixed cases) and every metacal type, so a first-component-only guard in get_mcal_flags would leave a g2-only NaN/inf clean. test_process_counts_flagged_fits_across_batches drives Ngmix.process through real batched saves with known fitter outcomes and asserts the run-health error line only appears when every fitted object is flagged, closing the gap where n_flagged going disconnected from the fit loop left the whole suite green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ngmix_runner: restore Martin's PSF-then-image all-empty guards 6272edc removed both of 84e40a5's runner-level early returns for a wholesale-empty tile (all PSF entries {}, or all galaxy entries "empty"), moving the check into Ngmix.process's per-object loop so the empty catalogue and run-health log still got written. But the first of those returns was a crash guard: it must run before anything reads the galaxy vignette store, because unpickling that store's many-epoch arrays can trigger a C-level malloc crash. Nobody reproduced the crash, but the original ordering was deliberate and a malloc crash kills a campaign job, so restore it instead of trusting that Ngmix.process's later read is safe. Both guards are back in ngmix_runner, at their original position and in their original order (PSF first), with Martin's comment intact. Instead of returning silently, each now writes the same five-HDU empty catalogue and run-health error line that a zero-fitted Ngmix.process run produces, via a new write_empty_tile_output helper -- so #879's completeness check still sees a catalogue for a wholesale-empty tile, without ever constructing an Ngmix instance (which would open the galaxy vignette store) to get it. write_empty_tile_output, and the empty_metacal_output/write_ngmix_fits helpers it's built from, are shared with Ngmix.compile_results/save_results so both paths produce byte-identical empty catalogues from one source. 6272edc's other change -- checking an object's PSF entry before its galaxy entry inside Ngmix.process's per-object loop, to skip allocating the galaxy stamp when the PSF alone is already empty -- is a genuine per-object optimisation for a tile that is only partly empty, independent of the tile-wide crash guard, and is kept. Added a test asserting the PSF-all-empty guard never opens the galaxy store: the galaxy store is a corrupted (non-sqlite) file, so opening it would raise. Checked by hand that swapping the two guards' order turns this red with the expected sqlite3.DatabaseError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ngmix, make_cat: mark metacal failures with ngmix's LM_FUNC_NOTFINITE Drop the ShapePipe-only FLAG_NO_RESULT = 2**30. get_type_flags keeps a fitter's own flags and maps every case with no finite shear -- a type reporting flags == 0 without a finite g, a result without flags, an absent type, and hence an object ngmix never fit -- to ngmix.flags.LM_FUNC_NOTFINITE (2**12). Every flag column now carries only ngmix's own bits. The never-fit contract folds into make_cat's failure-sentinel site, and astra.yaml's catalogue_assembly.failure_sentinels rationale records that never-fit rows fail the MCAL_FLAGS == 0 cut rather than passing it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uemfjv9ybCwtKtprksZbVY --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
cailmdaley
marked this pull request as ready for review
September 28, 2026 16:29
cailmdaley
added a commit
that referenced
this pull request
Sep 28, 2026
… feat/wire-external-masks Resolved by keeping develop's side everywhere #879's pre-squash content conflicted, then re-applying this branch's own changes (a846e08..dc11dab): - astra.yaml: the mask-cut decision moves into the masking sub-analysis as masking.mask_default_cut, in the #875 format (no Anchor sentence; the make_cat contract tag cites it). masking.sky_mask_application now selects catalogue_columns, since the committed config sets MASK_EXT_PATHS; config_tile_Mc.ini's MASK_EXT_PATHS carries its tag. universes pinned. - cfis_image_sims/config_tile_Mc.ini: rebuilt from develop's cfis copy (spread model gone), still without MASK_EXT_PATHS or per-epoch slots. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cailmdaley
added a commit
that referenced
this pull request
Sep 28, 2026
… feat/retire-versioning Resolved by keeping develop's side where #879's pre-squash content conflicted, then re-applying this branch's own changes (a846e08..bfb7efc). Two by hand: docs/source/pipeline_canfar.md stays retired (develop's one-line path fix goes with it), and the rebuilt tutorial drops the spread-model step that #911 removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cailmdaley
added a commit
that referenced
this pull request
Sep 28, 2026
Brings in #879 (campaign merges), #911 (spread model removed), #854, #905 and #923. - run_config: input_types:/machines: table resolver kept; develop's required `run:` and unresolved-$ check folded in (now over every non-table key). - create_final_cat.read_data keeps develop's raise on a missing column; the branch's skip-missing path is dropped (final_cat_merge is the one producer of final_cat_<run>.hdf5). - final_cat.param carries both NUMBER and TILE_UNIQUE_ID. - DAG tests follow the committed tile_detection default (data plans tile_get_catalogue); params pin regenerated for the tile_detect switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DksuyF9YrZAwHQeAMXsxp4
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.
Stacked on #894 and #875; needs #905 (fixed per-epoch slots). Closes the merge half of #890.
Campaigns don't yet produce the two merged files sp_validation reads:
clean_exposuredeletes the exposure store, and only per-tilefinal_catreachesproducts_dir. This PR adds:exp_persist(betweenexp_psfandclean_exposure): one tar per exposure,<products_dir>/exp/<shard>/<exp>/psf/<exp>.tar, plus a manifest with member SHA-256s. It always packsvalidation_psf;persist_exp:adds more (default[psf_model]). Skipped underpsf_model: fake.star_cat_merge: allvalidation_psf→<products_dir>/full_starcat_<run>.hdf5. A reclaimed exposure enters through its tar, so the merge never re-downloads it.final_cat_merge: all tiles'final_cat→<products_dir>/final_cat_<run>.hdf5, exactly the columns offinal_cat.param, for data and sims (replaces clean up workflow image sims #894'smerge_final_cats). The param file lists the 12 per-epoch slots (EXP_ID_n,CCD_n,HSM_*_PSF_n).Both merges are incremental: they read only added units, drop removed ones, rebuild on a column change, refuse a column whose dtype differs between tiles, and stamp the code snapshot as HDF5 attributes.
run:is required and names the campaign (file names, HDF5 group). Parse-time refusals:psf_model: mccd(persistence reads PSFEx products only), a cleaner enabled while scratch and products share one root, and any config path with an unexpanded$var. Never-fit rows pass through; consumers cut onNGMIX_N_EPOCH > 0.Checks. Contracts in
workflow/CONTRACTSand@scdocstrings.tests/workflow/drives the Snakefile on a throwaway campaign: rule set per input mode,clean_exposurewaiting on the persist manifest, product paths, the parse-time refusals of a missingrun:and of MCCD, and a params pin that fails when any rule'sparams.prechanges (--update-params-pin).test_final_cat_merge_invariants.pycovers the merged column set, per-epoch tuples, never-fit rows, and the missing-column and dtype refusals.Launch new campaigns on this; don't resume old ones. Every rule's
params.prechanges, so a resumed campaign reruns every finished unit.Verified: suite green in the dev image and CI;
exp_persistend-to-end on smk-m2 (127/127 exposures); dry runs for data+psfex (31 jobs) and sims+fake (28).🤖 Generated with Claude Code