Skip to content

Consistent use of the external healsparse masks - #847

Merged
cailmdaley merged 19 commits into
feat/snakemake-orchestrationfrom
feat/healsparse-external-masks
Sep 6, 2026
Merged

cailmdaley merged 19 commits into
feat/snakemake-orchestrationfrom
feat/healsparse-external-masks

Conversation

@cailmdaley

@cailmdaley cailmdaley commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Closes #846. Closes #837. Part of the #845 epic; builds on #852 and merges after it.

Until now, ShapePipe made its own masks: the mask module rasterized GSC star positions, Messier/NGC regions, and the instrument flags into a per-image flag file, and the star selection cut on that image. The mask force's external masks never entered the pipeline.

This PR removes all mask generation from ShapePipe. Two kinds of mask input remain, ingested differently.

  1. The instrument flags delivered with each exposure (bad columns, saturation) — the only masks that touch pixels. They flow as before: SExtractor reads them per CCD and records them per object as IMAFLAGS_ISO; ngmix sees them in its stamp weights.
  2. The UNIONS healsparse masks, fixed on the sky — never rasterized. Each map is queried once per object at its (RA, Dec) and the answer becomes a catalogue column: make_cat writes every bit, per band, into MASK_<band>, unfiltered. All selection on these columns happens downstream.

PSF star selection — the one decision that cannot be redone later — cuts only on the instrument flags: they mark corrupted measurements, stars whose own photometry and shape are unreliable. For everything the sky masks describe, the pipeline deliberately starts by relying on PSFEx's outlier rejection alone, and the first runs measure how well that holds. If it proves insufficient, there is machinery to pass any of the external masks — for example the star mask — to the PSF star selection, switched on in config.

Shape measurement uses no external mask: ngmix zero-weights instrument-flagged pixels and drops heavily flagged epochs, exactly as before. Whether an object in a halo, a trail, or a masked star enters the science sample is decided downstream from the MASK_<band> columns.

The survey footprint is a separate, positive coverage map — the exposure map of CCDs with a PSF model (#797), or the pointing-coverage map — and the mask bits subtract from it to give the final footprint.

The mask products

Sky-fixed boolean healsparse maps, one per bit, nside 131072 ≈ 1.6″; current products mask_ugriz_nside131072_n<bit>.hsp (2026-08-05, post-GSC2-fix; locations on the wiki):

bit value meaning
0, 1 1, 2 faint / bright star halo
2 4 star mask (the star's core and spikes)
3 8 manual mask (large galaxies)
4–8 16–256 no u g r i z data
9 512 outside the tile's unique region
10 1024 MaxiMask (trails, dead columns; tile-level)
11 2048 no Pan-STARRS z2 data

Paths and bit choices live only in config, so regenerated products cost no code.

What changed in the code

The mask module is deleted, together with everything that existed only to feed it: the GSC star-catalogue machinery, the WeightWatcher configs, and the tile-level masking variants. random_cat is deleted too — randoms and unmasked area are map algebra on the coverage map downstream, which is what closes #837. Exposure SExtractor now reads the instrument flag file straight from split_exp, and tiles are detected without any flag image. In the Snakemake workflow the exposure chain shrinks to get-images → split → psf: the star-catalogue staging, its config keys, and the mask rules are removed. The old CANFAR bash drivers still name the deleted configs; they are superseded by the workflow and left for a separate cleanup.

The first run of this branch refreshes the #850 old-vs-new comparison against the 2026 products.

— Claude on behalf of Cail

cailmdaley added a commit that referenced this pull request Jul 16, 2026
…images

New mask_ext module (PRD in #847): reads the image WCS, evaluates the
pixel grid in memory-bounded row chunks, queries the healsparse map,
applies a config-driven bit->flag mapping, optionally sums the external
instrument flag (matching Mask._build_final_mask semantics), and writes
the standard int16 <PREFIX>_flag<num>.fits. Same module serves tiles and
exposure CCDs; mask files and bit meanings live only in config. Example
tile/exposure configs; 7 unit tests on synthetic maps (bit mapping,
chunk-seam independence, RA wrap, off-map, ext-flag sum, WCS round-trip).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CfRuCQa2UHo44yp2MZDsJX
@cailmdaley
cailmdaley marked this pull request as ready for review July 16, 2026 17:00
@cailmdaley cailmdaley linked an issue Jul 16, 2026 that may be closed by this pull request
@cailmdaley

Copy link
Copy Markdown
Contributor Author

Policy update from the 2026-07-21 mask-force telecon (details on #845): the rasterizer design here is unchanged, but the BIT_FLAG_MAP policy is now consumer-aware. Star-halo bits (1, 2) must not enter the flags that setools' IMAFLAGS_ISO==0 cut sees for PSF star selection — halo masks flag, they don't reject. Defect bits (e.g. MaxiMask/streaks) may still cut. The issue body's PSF-star-selection paragraph has been updated accordingly; provisional on the surviving-star overlay check (#850).

— Claude (fable) on behalf of Cail

@martinkilbinger martinkilbinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all pretty good.

One question: Why is only the footprint mask ("BIT_FLAG_MAP = 64:1") in the config files? Should these not also include the star, maximask, and manual masks?

@cailmdaley cailmdaley changed the title Consume external healsparse masks: rasterize to pipeline flag images + per-band catalogue flags Consume external healsparse masks by per-object query at the catalogue level; instrument flags remain the only per-exposure input; no rasterization Jul 30, 2026
cailmdaley added a commit that referenced this pull request Aug 31, 2026
…images

New mask_ext module (PRD in #847): reads the image WCS, evaluates the
pixel grid in memory-bounded row chunks, queries the healsparse map,
applies a config-driven bit->flag mapping, optionally sums the external
instrument flag (matching Mask._build_final_mask semantics), and writes
the standard int16 <PREFIX>_flag<num>.fits. Same module serves tiles and
exposure CCDs; mask files and bit meanings live only in config. Example
tile/exposure configs; 7 unit tests on synthetic maps (bit mapping,
chunk-seam independence, RA wrap, off-map, ext-flag sum, WCS round-trip).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CfRuCQa2UHo44yp2MZDsJX
@cailmdaley
cailmdaley force-pushed the feat/healsparse-external-masks branch from c25cfdf to f6b9461 Compare August 31, 2026 12:59
cailmdaley and others added 5 commits August 31, 2026 09:05
First step of the external-healsparse-mask path (#846): the reader
rasterizes maskforce healsparse products onto image pixel grids in
place of internal mask generation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CfRuCQa2UHo44yp2MZDsJX
…images

New mask_ext module (PRD in #847): reads the image WCS, evaluates the
pixel grid in memory-bounded row chunks, queries the healsparse map,
applies a config-driven bit->flag mapping, optionally sums the external
instrument flag (matching Mask._build_final_mask semantics), and writes
the standard int16 <PREFIX>_flag<num>.fits. Same module serves tiles and
exposure CCDs; mask files and bit meanings live only in config. Example
tile/exposure configs; 7 unit tests on synthetic maps (bit mapping,
chunk-seam independence, RA wrap, off-map, ext-flag sum, WCS round-trip).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CfRuCQa2UHo44yp2MZDsJX
… maps

Optional MASK_EXT_PATHS (band:path pairs, env-expanded) in the make_cat
section: each band's healsparse map is queried at object world positions
(XWIN_WORLD/YWIN_WORLD) and added as a MASK_<BAND> column — the ShapePipe
end of UNIONS-WL/spherex#38. Strict no-op when unset; off-map objects
carry the map sentinel (-1 for integer maps). 3 unit tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CfRuCQa2UHo44yp2MZDsJX
The real 2025 r-band UNIONS mask (mask_r_nside131072.hsp) is a boolean
healsparse map (True = masked), not an integer bit-flag map. With a
boolean map, any BIT_FLAG_MAP bit other than 1 silently selects nothing
(True & 64 == 0), producing an all-clean flag image. Raise instead, and
document the two mask flavours in the example configs.

Found by the real-data smoke: rasterizing the candide mask copy onto
the CFIS.233.293 tile grid.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013btLTHCgmiiggZmxJ4hM3n
The catalogue-level mask query path (parse_mask_ext_paths,
save_mask_ext_data) added in this PR had no shipped example. Document
MASK_EXT_PATHS in config_make_cat_psfex.ini, the tile-level
MAKE_CAT_RUNNER config, matching the design language used for the
rasterizer's BIT_FLAG_MAP in config_tile_MaExt.ini.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KQnUyBXB85PdFF4xAKC6WC
cailmdaley and others added 11 commits August 31, 2026 10:45
ShapePipe stops making masks (#846, part of #845). Gone in one piece: the
`mask` module (GSC 2.3 Vizier queries, Messier/NGC region files, the
WeightWatcher .ww default and the halo/star .reg templates), the `mask_ext`
rasterizer that briefly stood in for it, and every config that ran either —
the *.mask / *.mask_simu mask configs, config_{exp,tile}_Ma_onthefly.ini,
config_{exp,tile}_MaExt.ini, the defunct MaMa and Sx_exp_* variants, and
workflow/config/cfis/config_exp_Ma.ini with its three dangling symlinks.

The star-catalogue staging the mask module needed goes with it: the
`star_catalogue` / `exp_star_cat` Snakemake halves (workflow/scripts/star_cats.py),
scripts/python/create_star_cat.py and collate_star_cat.py, and
shapepipe.utilities.vizier. Two utilities become dead in the same stroke and
are deleted rather than left orphaned: `focal_plane` (only create_star_cat and
star_cats.py called focal_plane_disc) and `utilities.file_io` (write_atomic had
exactly those two callers; it is prose-cited from tile.smk, fixed there).

Nothing replaces them with code. The sky-fixed masks are healsparse maps and
are queried per object — MASK_<band> columns in make_cat, FLAG_EXT in the new
mask_query module — so what used to be a pipeline stage is now a config entry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
…e masks

One healsparse lookup, two consumers. shapepipe.utilities.mask_query holds the
primitive: `query_map` reads a map and returns its value at each (RA, Dec), and
`flag_positions` ORs several maps into one integer per object.

`make_cat` now calls query_map for its per-band MASK_<band> columns instead of
opening healsparse itself, and the new `mask_query` module runs between
sextractor and setools on exposures, writing an integer FLAG_EXT (0 = clean)
into a NEW sexcat_ext<num>.fits beside the input — the spread_model `add`
pattern, so the LDAC_IMHEAD HDU survives and nothing mutates in place.
star_selection.setools gains `FLAG_EXT == 0` beside every `IMAFLAGS_ISO == 0`:
setools expressions have no bitwise operators, so the bit selection happens in
the module (MASK_PATHS, optional MASK_BITS) and the config only tests for zero.

The one interpretive choice is off-coverage. get_values_pos returns a map's
sentinel outside its coverage — False for boolean maps, -1 for integer ones.
make_cat passes that through verbatim (its documented off-map flag), but
flag_positions treats it as NOT flagged for both kinds: OR-ing -1 in would flag
every object a map does not reach, i.e. a map whose footprint stops short of an
exposure would silently reject all of its stars. Off-coverage counts are logged.

Tests build tiny nside_sparse=4096 maps and a three-HDU LDAC fixture and lock
in the boolean/integer/MASK_BITS/OR/off-coverage cases, that the LDAC structure
and the input file survive, and that make_cat's verbatim behaviour is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
Exposures. SExtractor now reads the instrument flag straight from split_exp
(`FILE_PATTERN = image, weight, flag` — split_exp already writes an unprefixed
flag<num>.fits per CCD), with the run_sp_exp_Ma INPUT_DIR entry gone;
mask_query_runner sits between it and setools in both the example and the
workflow config, and star_selection.setools cuts on FLAG_EXT == 0 beside every
IMAFLAGS_ISO == 0. Same rewiring in config_exp_mccd.ini, which shares that
setools file. sextractor_runner's declared input_module follows the real parent.

Tiles. config_tile_Sx_nomask.ini becomes THE tile config: tiles have no
instrument flag image and no mask module now, so FLAG_IMAGE = False with
default_noimaflags.param is the only variant, in example/cfis and in the sims
dir. random_cat still needs a pixel mask image, but nothing in the pipeline
produces one — its second input is now declared external (the healsparse-native
replacement is #797), which is a real capability gap and is flagged as one in
the config, the runner and docs/source/random_cat.md rather than papered over.

Snakemake. `exp_psf` now depends on `exp_split` directly; `star_catalogue`,
`exp_star_cat`, `exp_mask`, `star_cat_cmd`, `in_container` and the STAR_CAT_*
helpers are gone, along with `config["star_cats"]` and the two mid-chain
localrules. The `exp_short` group goes too, and the docstring says why: it
existed to fuse exp_split with exp_mask, and a group of one rule submits exactly
the job the ungrouped rule submits. completeness.py trades its exp_mask stage
(mask_runner 40/1) for a mask_query_runner row inside exp_psf, floor 2 rather
than 40 because setools tolerates the same sparse-CCD attrition either side of
it; run_report drops the two stages; clean_exposure stops reclaiming link farms
that no longer exist.

Verified in the container: all 30 module runners import, every example and
workflow config's MODULE list resolves through get_module_runners, and the
workflow parses and builds its DAG (`snakemake --lint`, `-n`).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
pipeline_tutorial.md's "Mask images" section becomes "Masks" and explains the
design instead of the old two-run mask procedure: healsparse maps queried per
object into FLAG_EXT and MASK_<band>, the instrument flag as the one mask that
reaches pixels, no internet access and no star-catalogue download anywhere.
pipeline_canfar.md loses its mask-tiles and mask-exposures steps and the
combine_runs flag_* staging; random_cat.md gains a warning that its input mask
images now come from outside ShapePipe; workflow/README.md and the sims README
describe the chains as they now are.

Dependencies. weightwatcher leaves the Dockerfile and the three docs pages that
listed it — the deleted mask module was its only caller (`ww` appears nowhere
else). astroquery and hpgeom leave pyproject.toml: astroquery had exactly two
importers, utilities/vizier.py and star_cats.py, and hpgeom was the mask_ext
rasterizer's. `uv lock` regenerated the manifest (astroquery, html5lib, pyvo
removed; hpgeom stays, healsparse pulls it in). canfar_avail_results loses its
-m / pipeline_flag check mode, and scripts/README.rst its create_star_cat entry.

Left deliberately: shapepipe.utilities.summary{,_params_pre_v2}'s mask_runner
entries. Those describe the pre-v2 CANFAR job map and parse the logs of runs
that already exist on disk — removing them would break summary_run against
historical trees without making anything current cleaner.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
The diet is settled: instrument flags (IMAFLAGS_ISO, already read by
SExtractor) plus the UNIONS star-body map (bit 2,
mask_ugriz_nside131072_n4.hsp), and nothing else. The committed
[MASK_QUERY_RUNNER] examples in config_exp_psfex.ini (both copies) and
config_exp_mccd.ini now name exactly that one map instead of the two-map
placeholder.

Halo bits 0 and 1 stay out on purpose — halos flag objects for the final
catalogue, they do not reject PSF stars (mask-force telecon, 2026-07-21) — and
MaxiMask is out too. The module docstring says so under its own heading, so
that a reader who finds one path in the config knows it is a decision rather
than an unfinished list, and knows widening it is a config edit and no code.

That asymmetry also explains the contract: MASK_PATHS is a path list rather
than a bit mask because the UNIONS products are one boolean map per bit, so
choosing bits is choosing files. MASK_BITS survives for integer maps that pack
several bits into one file, and its commented example drops from 1028 to 4 to
match the diet. The setools header and the tutorial's Masks section carry the
same note.

Configs re-validated in the container (0 bad, all MODULE lists resolve) and the
10 mask tests still pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
… docs

Three follow-ups from review.

CI. deploy-image.yml's runtime binary smoke still ran `weightwatcher --version`
after the Dockerfile dropped the package, so the very next image build would
have failed on a tool nothing calls. Step line and the comment's mention both go.

SEGMENTATION, and this one was a real regression I introduced. Folding
config_tile_Sx_nomask.ini into config_tile_Sx.ini carried the nomask variant's
`CHECKIMAGE = BACKGROUND` over the masked config's `BACKGROUND, SEGMENTATION`.
The segmentation check image is not masking furniture: vignetmaker cuts
per-object segmentation stamps from it (config_tile_PiViVi_canfar_sx.ini
FILE_PATTERN = sexcat, segmentation), and ngmix's uberseg blend handling raises
at construction without them (ngmix.py ~496, shapepipe#776). Restored in both
copies. The sims config already had it, which is why nothing else caught this.

Docs. pipeline_canfar.md's "Collate star catalogues" section drove
collate_star_cat.py, deleted with the star-catalogue tooling. The recipe is
replaced with a warning that names what is missing rather than a silent cut: the
merge step below it consumes validation_psf_conv files that now have no producer
in this repo, and the star_cat/ + `combine_runs.bash -c psf_conv` staging around
it does not apply either. The PSF measurement itself is unchanged — shapes are
measured in sky coordinates during interpolation — so the collation is a
pass-through that is straightforward to rebuild. No other docs page referenced a
deleted script.

Re-verified in the container: 30/30 module runners import, 65 configs' MODULE
lists resolve (0 bad), the 10 mask tests pass, the workflow still parses and
builds its DAG, and deploy-image.yml is valid YAML.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
Five review items, all in the query path.

PARTIAL READS. query_map did HealSparseMap.read(path) on a 583 MB map, per CCD
— ~22 GB of I/O per 40-CCD exposure to answer questions about 0.06 deg². It now
reads HealSparseCoverage first, computes the coverage pixels the positions touch
with hpgeom.angle_to_pixel at the map's nside_coverage, and loads only those.
make_cat gets this for free, sharing the primitive.

Measured on the DR6 star map for 2000 positions in one CCD-sized box, one
process each: partial 0.102 s / 157 MiB peak RSS, full 12.8 s / 3364 MiB,
IDENTICAL values. 125x faster, 21x less memory. The full-read footprint is
3.3 GiB — under the 4 GB mark — and the partial read adds ~0.15 GB to a rule
already asking for 16 GB, so exp_psf's mem_mb is left alone. Per exposure this
is ~4 s of map reading instead of ~8.5 min. test_partial_read_matches_full pins
partial == full for both map dtypes.

Two things the reviewer's sketch did not anticipate, both found by probing the
real map rather than reasoning. (a) healsparse RAISES when no requested pixel is
in the coverage map, so the all-off-coverage case is answered without asking it:
one small probe read for dtype and sentinel, then the sentinel everywhere.
(b) The neighbours padding is insurance, not necessity — every position falls in
exactly one coverage pixel and all are requested — but at nside_coverage=128 it
costs under a megabyte, so it stays.

BOOLEAN OFF-COVERAGE. n_off was hardcoded to 0 for boolean maps, so a map that
misses an exposure logged "0 flagged, 0 outside coverage" — indistinguishable
from clean. valid_mask=True cannot fix this: the UNIONS products are BOOLEAN, and
healsparse stores only the True pixels, so valid_mask returns the value itself
(verified against mask_r_nside131072_n4.hsp). Coverage now comes from the
coverage mask, which works for both dtypes, and all-off-coverage logs a warning
saying the zero column means "the map does not reach here", not "clean".

Also: FLAG_EXT's docstring no longer claims the value says which bits fired —
true only for integer maps, since boolean maps can only contribute 1.
config_Rc.ini's mask INPUT_DIR placeholder becomes <path/to/tile_masks> rather
than a $SP_CONFIG path that does not exist. A zero-detection CCD no longer
raises on data["XWIN_WORLD"] — it writes an empty FLAG_EXT column, matching the
sparse-CCD tolerance setools and the completeness floor already carry. hpgeom
returns to pyproject.toml, now imported directly rather than via healsparse.
pipeline_canfar.md's HSM sky-coordinates paragraph is lifted out of the
collate_star_cat deletion warning into standing prose, pointing at the test that
pins the convention.

Verified: 15/15 mask tests pass, 30/30 runners import, 65 configs resolve, the
workflow still builds its DAG, uv.lock regenerated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
… masking

Scope error in eb93838, mine to own: collate_star_cat.py was swept up with the
GSC masking star catalogues on a name match. It is the PSF VALIDATION collation
— per-exposure validation_psf files gathered into the validation_psf_conv
catalogues config_Ms_psfex_conv.ini merges — and has nothing to do with mask
generation. Restored byte-identical from origin/feat/snakemake-orchestration,
with tests/module/test_collate_star_cat.py, and pipeline_canfar.md's "Collate
star catalogues" section put back verbatim in place of the deletion warning
ba2271a wrote (the HSM sky-coordinates paragraph returns with it, as the
section's own prose, which is where it started).

Two things asked for in the revert turn out not to be needed, checked rather
than assumed:

  * focal_plane.py and utilities/file_io.py stay deleted. collate_star_cat.py
    imports nothing from shapepipe at all — only stdlib plus tqdm, joblib,
    numpy, astropy, galsim and cs_util. focal_plane_disc and write_atomic had
    exactly two callers between them, create_star_cat.py and star_cats.py, and
    both are masking and both stay deleted.
  * There is no scripts/README.rst entry to restore. That file listed
    create_star_cat (the masking one) and never listed collate_star_cat, so the
    edit in 1bceb2d was already correct.

create_star_cat.py, star_cats.py and utilities/vizier.py remain deleted, as
intended.

Verified in the container: the 9 restored tests pass alongside the 15 mask ones
(24 total), 30/30 runners import, 65 configs resolve, the workflow builds its
DAG, and both restored files diff clean against origin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
…name

Three from final review.

`mask_map.sentinel` replaces `mask_map._sentinel` in the probe path — same
value (verified for both int and bool maps on healsparse 1.12.2), public API.

The probe also indexed `np.flatnonzero(coverage.coverage_mask)[0]` unguarded,
so a map with no coverage at all died with a bare IndexError from inside the
utility. It now raises `ValueError("healsparse map <path> has empty coverage")`,
naming the file, with a test.

The shipped MASK_PATHS placeholder becomes the real DR6 product name,
`mask_r_nside131072_n4.hsp`, in all three configs. The products are staged at
/project/6001537/cdaley/masks/dr6/ (same tree as
/project/def-mjhudson/cdaley/masks/dr6/), which is where the partial-read
measurement in f7d1fd6 was taken — that docstring already named the file
correctly, so nothing else mentioned the old placeholder.

25 tests pass in the container (mask_query, make_cat_mask_ext,
collate_star_cat); 65 configs resolve.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
…ra downstream

Cail's call. random_cat computed a tile's effective unmasked area and drew
randoms inside it by counting zero pixels in a tile mask IMAGE. That input was
the deleted mask module's output, and with the query design there is no such
image and no plausible producer for one: the survey window is map algebra on the
healsparse coverage map (#797), done downstream, not a per-tile pipeline step
that rasterizes to pixels first.

Rather than keep a module wired to a placeholder path nobody can fill — which is
what f7d1fd6 left, an honest but unpaid IOU — the module goes: the package, the
runner, config_Rc.ini, docs/source/random_cat.md and its toc.rst entry. There
were no tests to remove.

The module was already unsound independently of masking, which is part of why
keeping it had no value: `save_as_healpix` is called as `_save_as_healpix`, and
`process` references `file_name` and `output_dir` which are never bound. Any run
past the first few lines would have raised.

`reproject` leaves pyproject.toml with it — random_cat.py held its only import
(the two ngmix hits for "reprojection" are prose in comments). uv.lock
regenerated: reproject, plus pims, pyavm, slicerator, toolz and zarr that came
in behind it.

Verified in the container: 29 runners import (was 30), 64 configs resolve (was
65, 0 bad), the 25 mask/collate tests pass, the workflow still builds its DAG,
and random_cat / N_RANDOM / RandomCat / config_Rc / run_sp_Rc appear nowhere
outside scripts/sh/, which stays untouched as agreed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
… placeholder at the real ladder

The Aug-2026 post-GSC2 band-combined healsparse products (arc:home/mhudson/masks/)
use the mask_ugriz_nside131072_n<bit>.hsp naming; the mask_r_* files on canfar's
ShapePipe/mask are the 2025 vintage.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
@cailmdaley cailmdaley changed the title Consume external healsparse masks by per-object query at the catalogue level; instrument flags remain the only per-exposure input; no rasterization Consistent use of the external healsparse masks Aug 31, 2026
cailmdaley and others added 2 commits August 31, 2026 12:21
…hing by default

The PSF star selection no longer cuts on the external masks. mask_query stays in
the chain and keeps writing FLAG_EXT from the shipped MASK_PATHS (the star-body
map, bit 2); star_selection.setools drops `FLAG_EXT == 0` from all four mask
blocks and rejects on the instrument flags alone, leaning on outlier rejection
for the rest.

This makes the exposure side agree with the tile side, which was already
permissive: make_cat writes MASK_<band> unfiltered. Flag transparently, cut
downstream — so the effect of a mask on the star sample can be MEASURED before
it is imposed, rather than baked in on the way past.

The escape hatch is documented where someone would look for it, because a
permissive default is only safe if reversing it is obvious. star_selection.setools'
header now spells out the change (add `FLAG_EXT == 0` beside each
`IMAFLAGS_ISO == 0`, one line per block) and says why it exists: if outlier
rejection turns out not to be robust enough. The module docstring gains a
"Nothing cuts on it by default" section, and the config comments beside
MASK_PATHS say NOTHING CUTS ON IT rather than naming a cut that is no longer
there. The tutorial's Masks section, pipeline_canfar.md and workflow/README.md
lose the same stale claim; the shared utility now says setools *could* cut on
the column, not that it does.

The diet section is retitled: MASK_PATHS is a list of maps to RECORD against
each candidate, not to reject on. Halos stay out for the same reason as
before — they say nothing about whether a star is a good PSF sample.

Verified: 25 tests pass, 29 runners import, 64 configs resolve, ruff clean.
Only the four cut lines changed in the setools body — the mask blocks and their
IMAFLAGS_ISO tests are otherwise untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
…ASK_EXT

Two changes, both about making the naming and the default say what the design
means.

MASK_EXT, not FLAG_EXT. The column is one family with make_cat's MASK_<band>:
an external mask, queried and stored per object. "FLAG" belonged to the other
kind of mask entirely, which is the distinction the docstrings now lead with —
instrument flags mark a CORRUPTED MEASUREMENT (bad columns, saturation) and are
the only masks that reject anything in-pipeline, via IMAFLAGS_ISO in setools
and zero-weighted pixels and dropped epochs in ngmix; healsparse masks are
sky-fixed LOCATION flags that say where an object sits, not that its pixels are
broken, so they are queried to columns and every rejection is downstream.
Renamed in code, tests, docstrings, the setools header, the tutorial and both
READMEs.

MASK_PATHS ships commented out, and absence is now a strict no-op rather than a
ValueError: the runner treats a missing key the way make_cat treats a missing
MASK_EXT_PATHS, and MaskQuery with no maps copies the catalogue through
byte-for-byte with NO MASK_EXT column. The copy is the load-bearing part —
setools reads this module's output, so a no-op that wrote no file would break
the chain instead of disabling the query. mask_query stays in the MODULE chain
either way, so turning the query on is uncommenting one line and never editing
the chain. Two tests cover it, one asserting the output is byte-identical to
the input.

The framing that ties both: the PSF star selection deliberately starts from
outlier rejection alone, and MASK_EXT is the configurable pickup if that proves
insufficient — write the column, measure the effect, impose it only if the
measurement says to.

Also corrected in passing: edec546 renamed the measured file in the
partial-read note along with the config placeholders, which credited the
measurement to a file I did not run against. The note now says what was
actually measured — the staged single-band copy mask_r_nside131072_n4.hsp — and
names the mask_ugriz ladder it belongs to.

Verified: 27 tests pass, 29 runners import, 64 configs resolve (MASK_PATHS
absent, section and chain entry present in all three), ruff clean on the
touched files.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
@cailmdaley

Copy link
Copy Markdown
Contributor Author

Looks all pretty good.

One question: Why is only the footprint mask ("BIT_FLAG_MAP = 64:1") in the config files? Should these not also include the star, maximask, and manual masks?

Question moot now, BIT_FLAG_MAP was a holdover from when the masks were getting rasterized onto the image grids. All bit masks are now consistently written to the catalog.

@cailmdaley
cailmdaley merged commit 8ce3416 into feat/snakemake-orchestration Sep 6, 2026
2 checks passed
@cailmdaley
cailmdaley deleted the feat/healsparse-external-masks branch September 6, 2026 22:21
cailmdaley added a commit that referenced this pull request Sep 9, 2026
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 added a commit that referenced this pull request Sep 26, 2026
PR #847 removed ShapePipe's mask generation and left the sky-fixed UNIONS
masks to be queried per object by make_cat via MASK_EXT_PATHS. The committed
config never set it, so no campaign wrote a mask column and the merged shear
catalogue carried no mask information at all.

It does now. The DR6 Aug-2026 ugriz bit ladder is staged group-readably at
/project/def-mjhudson/unions-wl/masks/dr6-2026-08 (11 boolean healsparse maps,
nside 131072, True = masked), reached as a third input root SP_INPUT_MASKS
beside tiles and exposures. config_tile_Mc.ini names all 11 in MASK_EXT_PATHS
and carries the bit table; final_cat.param carries the matching MASK_n1 ..
MASK_n2048, replacing the IMAFLAGS_ISO note.

Two caveats live with the config, because a cut written without them is wrong:
the August regeneration swapped the content of bits 0 and 1, so n1|n2 is only
safe combined; and n2048 is 1 where there is no Pan-STARRS z2 data, so an OR
over every column masks the whole survey. Nothing cuts on these here.

PSF-star selection is untouched: MASK_PATHS in config_exp_psfex.ini stays
commented out.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
cailmdaley added a commit that referenced this pull request Sep 26, 2026
PR #847 removed ShapePipe's mask generation and left the sky-fixed UNIONS
masks to be queried per object by make_cat via MASK_EXT_PATHS. The committed
config never set it, so no campaign wrote a mask column and the merged shear
catalogue carried no mask information at all.

It does now. The DR6 Aug-2026 ugriz bit ladder is staged group-readably at
/project/def-mjhudson/unions-wl/masks/dr6-2026-08 (11 boolean healsparse maps,
nside 131072, True = masked), reached as a third input root SP_INPUT_MASKS
beside tiles and exposures. config_tile_Mc.ini names all 11 in MASK_EXT_PATHS
and carries the bit table; final_cat.param carries the matching MASK_n1 ..
MASK_n2048, replacing the IMAFLAGS_ISO note.

Two caveats live with the config, because a cut written without them is wrong:
the August regeneration swapped the content of bits 0 and 1, so n1|n2 is only
safe combined; and n2048 is 1 where there is no Pan-STARRS z2 data, so an OR
over every column masks the whole survey. Nothing cuts on these here.

PSF-star selection is untouched: MASK_PATHS in config_exp_psfex.ini stays
commented out.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
cailmdaley added a commit that referenced this pull request Sep 26, 2026
PR #847 removed ShapePipe's mask generation and left the sky-fixed UNIONS
masks to be queried per object by make_cat via MASK_EXT_PATHS. The committed
config never set it, so no campaign wrote a mask column and the merged shear
catalogue carried no mask information at all.

It does now. The DR6 Aug-2026 ugriz bit ladder is staged group-readably at
/project/def-mjhudson/unions-wl/masks/dr6-2026-08 (11 boolean healsparse maps,
nside 131072, True = masked), reached as a third input root SP_INPUT_MASKS
beside tiles and exposures. config_tile_Mc.ini names all 11 in MASK_EXT_PATHS
and carries the bit table; final_cat.param carries the matching MASK_n1 ..
MASK_n2048, replacing the IMAFLAGS_ISO note.

Two caveats live with the config, because a cut written without them is wrong:
the August regeneration swapped the content of bits 0 and 1, so n1|n2 is only
safe combined; and n2048 is 1 where there is no Pan-STARRS z2 data, so an OR
over every column masks the whole survey. Nothing cuts on these here.

PSF-star selection is untouched: MASK_PATHS in config_exp_psfex.ini stays
commented out.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
cailmdaley added a commit that referenced this pull request Sep 26, 2026
PR #847 removed ShapePipe's mask generation and left the sky-fixed UNIONS
masks to be queried per object by make_cat via MASK_EXT_PATHS. The committed
config never set it, so no campaign wrote a mask column and the merged shear
catalogue carried no mask information at all.

It does now. The DR6 Aug-2026 ugriz bit ladder is staged group-readably at
/project/def-mjhudson/unions-wl/masks/dr6-2026-08 (11 boolean healsparse maps,
nside 131072, True = masked), reached as a third input root SP_INPUT_MASKS
beside tiles and exposures. config_tile_Mc.ini names all 11 in MASK_EXT_PATHS
and carries the bit table; final_cat.param carries the matching MASK_n1 ..
MASK_n2048, replacing the IMAFLAGS_ISO note.

Two caveats live with the config, because a cut written without them is wrong:
the August regeneration swapped the content of bits 0 and 1, so n1|n2 is only
safe combined; and n2048 is 1 where there is no Pan-STARRS z2 data, so an OR
over every column masks the whole survey. Nothing cuts on these here.

PSF-star selection is untouched: MASK_PATHS in config_exp_psfex.ini stays
commented out.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
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>
cailmdaley added a commit that referenced this pull request Sep 28, 2026
…st, star_cat_merge, final_cat_merge) (#879)

* feat(orchestration): the keep list, and the script that acts on it

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>

* feat(orchestration): exp_persist, between exp_psf and clean_exposure

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>

* docs(orchestration): exp_persist in the rule list, and why the report 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>

* feat(orchestration): exp_persist packs one tar per exposure, not loose 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

* fix(orchestration): persist_exp never leaves a .tmp behind on failure

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

* docs(orchestration): drop references to the removed star-catalogue rules

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

* feat(orchestration): the two campaign-level merges

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

* fix(orchestration): seven defects in the campaign-level merges

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

* fix(create_final_cat): make the merged-catalogue writer reproducible

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

* perf(orchestration): size the two merges from the data, not from a guess

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

* feat(orchestration): persist_exp keeps NAMED products, not globs

Closes the readability half of CosmoStat/shapepipe#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

* perf(merge_starcat): accumulate arrays, not python lists of floats

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

* feat(orchestration): the star catalogue's inputs are not a user choice

`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

* fix(cfis): two stale columns in final_cat.param, and no mask column at 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 f0fca23e
called it in June and what the catalogues carry.

NGMIX_NEIGHBOUR_FLAG STAYS. It was added to make_cat in fa6e0016 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

* perf(merge_starcat): two passes, so nothing is held twice

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

* feat(orchestration): final_cat_merge reconciles instead of rebuilding

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

* fix(orchestration): eight defects found reviewing the merge work

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

* feat(orchestration): the star catalogue becomes hdf5, reconciled like 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

* fix(orchestration): nine findings from the third review

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

* test(unit): property-based state machines for reconcile and persist_exp

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

* fix(persist-exp): the PSF run dir is run_sp_exp_SxSePsf

persist_exp.py carries its own copy of the exposure PSF stage's run-dir name,
and it still held the pre-98bc0857 spelling. exp_persist would have searched
run_sp_exp_SxSePsfPi, found nothing, and tarred an empty product set -- a
silent loss rather than a failure, since an exposure with no keepable products
is a legitimate state.

This half of the rename lives here rather than in the develop hotfix because
persist_exp.py does not exist on develop; it arrives with this branch.
tests/unit/test_workflow_run_names.py (in the hotfix) checks this file when it
is present and skips the check when it is not, so the guard travels with
whichever branch has something to guard.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
(cherry picked from commit 5c1d41fe31537c6d22628670de4be8083ded5beb)

* 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
e32c4fb9). 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

* final_cat_merge, star_cat_merge: record code provenance in the HDF5 attributes

sp_validation opens the campaign's merged catalogues without any link back to
the code that built them; a `final_cat_<campaign>.hdf5` or
`full_starcat_<campaign>.hdf5` produced under one commit is indistinguishable
from one produced under another. `sp run` already snapshots that information
per campaign (bin/sp's `$STATE_DIR/code/snapshot.json`), so this wires it into
the two merge rules: hdf5_reconcile.code_provenance() reads the snapshot (or
says "unknown" when the workflow runs outside `sp run`) and apply() stamps
code_head/code_branch/code_dirty/code_snapshot_at, plus a newline-joined
code_dirty_files when the snapshot was dirty, onto the output file's root —
written only when the file is actually rewritten, matching the existing
no-op-leaves-the-mtime-alone contract.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* 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 o…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consume external healsparse masks: rasterize to pipeline flag images + per-band catalogue flags Extend size of bright star masks [BUG]

2 participants