Snakemake orchestration for the real-data pipeline - #852
Open
cailmdaley wants to merge 101 commits into
Open
Conversation
) Implements the round-2 design in #848: static two-phase execution driven by a parse-time SQLite run index (never a rule input, so appending tiles never invalidates completed work); rules wrap the existing per-unit shapepipe_run config chains at tile/exposure granularity with deterministic RUN_DATETIME=False directory() outputs; in-job completeness count-floor (the ported bash complete_check table, workflow/scripts/completeness.py); one-allocation execution profile for nibi with apptainer software-deployment (the workflow never calls apptainer itself); out-of-DAG run_report verb + onsuccess/onerror hooks; workflow/bin/sp as the committed launcher. First pass touches no module code. Validated by dry-run against a 4-tile P3 subset (219-job compute DAG); prepare phase runs green on nibi. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s.cmd
Snakemake formats params once, so a {threads} placeholder inside params.cmd
survives literally and SMP_BATCH_SIZE never resolves (fork width silently 1,
serial modules — the p3-first10 failure mode). Move '--threads {threads}' into
each rule's shell: string; tile_exp_forest takes none (build_forest.py is
single-threaded symlinking and has no such flag). Completeness table doc
touch-ups ride along.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NA8M1oLZLbWJxwoyTNrTAi
…star-cat store naming Three P0-blocking fixes found by running against real data: - exposures(name): the fabricated per-unit exp_numbers list must carry the original exposure name verbatim (2605805p — get_images matches <name>.fits.fz; the bare dedup id matches nothing). The index now stores both; exp_get_images passes --exp-name from the parse-time dict. - tile_star_cat: the pre-generated store names tiles in ShapePipe's image-number convention (dots->dashes); translate when linking. - profile: PYTHONPATH pinned to this branch's src/ (identical to develop@97e16d50 — orchestration commits never touch src/); NOT shapepipe-prod (drifted to a PR branch mid-run, and the live p3-batch1 job reads it) and not the sif default (frozen pre-#843). Run scoped to the 210/211 overlap quad (19 unique exposures, 15 shared — exercises structural dedup; the zero-overlap 196 quad is kept for the append-invariant test). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NA8M1oLZLbWJxwoyTNrTAi
… config - get_images looks up <name>.fits.fz: the store keeps the CFHT 'p' suffix the index's base-id key strips. exposures gains a name column; rules pass --exp-name; the wrapper writes the ORIGINAL name into the fabricated exp_numbers list — and writes it unconditionally (an exists-guard pinned a stale pre-fix file). - Star cats are consumed as DIRECTORIES via the wrapper's $SP_RUN symlinks (the v2.0 mechanism; exp cats are per-CCD, 40/exposure) — dropped the per-unit star-cat file rules that linked nonexistent names nothing read. - get_images completeness counts follow the nibi symlink configs (tile 2, exp 3; the v2.0 4/6 were the canfar vos flavor), verified against p3-batch1. - P0 run config: 210/211 quad (19 unique exps, 13 shared across tiles — exercises structural dedup; the 196 quad had zero overlap), PYTHONPATH pinned to this branch's src (= develop@97e16d50; shapepipe-prod drifted mid-run). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iterally
Same single-format trap as {threads} (187950c): Snakemake formats the shell
string once, so the escaped {{output}} in tile_exp_forest's params reached
build_forest as a literal './{output}' — all four forest jobs racing one
garbage dir (FileExistsError at 50% of the P0 run). Move --forest {output}
into the shell string. Swept the rules for further escaped placeholders: none.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ac/code/session_01NA8M1oLZLbWJxwoyTNrTAi
…pe is not cosmetic exp_utils.get_exp_output_files hardwires the v2.0 sharded layout into its $SP_EXP glob (<SP_EXP>/<prefix>/<base>/output/run_sp_*/...); a flat forest fails every tile gather stage with 'No split_exp_runner output found'. The exposure STORE stays flat (nothing globs it — the index drives every path); only the module-facing forest view carries the 2-digit shard. Verified by hand-running tile_merge_headers on 210.296 (1/1 OK). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ule's params The default rerun-triggers' code trigger hashes only the rule's own shell string — NOT external scripts it calls. Bitten live: the forest-shard fix changed build_forest.py but no rule text, so stale flat forests survived a rerun and tile_merge_headers kept failing. SCRIPT_HASH (md5 of sp_rule.py + completeness.py + build_forest.py, computed at parse time) rides into params via an accept-and-ignore --script-hash flag, so script edits propagate exactly like code edits under the full default triggers. One-time cost: every rule's params change once, re-running completed work — the same tradeoff as any code change under full triggers, and the P0-scale rerun doubles as a live test of rerun semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-full quota) def-mjhudson /project sits at its 27/27 TiB ceiling: a .snakemake metadata write died mid-run (killing an otherwise-green invocation after all 8 ngmix chunks of the first tile passed), and by end of night even git and file edits on /project failed. bin/sp now runs snakemake with --directory <run_dir>-state on /scratch (state only — data paths are absolute), the Snakefile resolves its configfile relative to itself so --directory can't break it, and the run index lives with the run on /scratch until project space is reclaimed (finding 15's /project placement stays the design intent; noted in config). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The per-unit star_cat file rules were removed (the store is per-CCD and the
mask config reads it as a dir symlink), but exp_mask kept declaring
exp/{exp}/star_cat-{exp}.fits as input. No producer exists, so the main DAG
became unbuildable the moment the tile list grew past the prepared set —
MissingInputException on every new exposure. Dormant for P0 (all 19 exposures
pre-staged); caught live by the append-invariant test (196 quad appended:
DAG build died before scheduling anything).
With the fix the append dry-run schedules exactly the appended cone (185 jobs:
30 new exposures x4 stages once each, 4 tile chains) and nothing for the
finished tiles; restored-state dry-run returns to 'Nothing to be done'.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0188niYp9MmhLmZou1KXWbye
Contributor
Author
|
P0 delivered: 4/4 tiles through — Claude (Fable) on behalf of Cail 🤖 Generated with Claude Code |
…tion reporting sextractor writes 3 files/CCD on nibi (120/exp, not v2.0's 80); vignetmaker run_2 writes 5 sqlites/tile (not canfar's 4) — both verified against the P0 tree and the bash baseline. run_report now aggregates found-vs-expected at file granularity per runner, so warn-runner attrition (psfex_interp) is reported instead of hidden behind unit-level completeness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRBKnHV6oUXEqRpsjqJpex
…he only mode The three module touches named in #848: - ID_OBJ_MIN/MAX read via getexpanded, so the committed ngmix template can interpolate $NGMIX_ID_MIN/$NGMIX_ID_MAX per chunk. Unset vars fail loudly. - SEED_FROM_POSITION is retired as an option: each object's RNG derives from its own (ra, dec, ccd), so results are bit-identical under any chunking. Tile-seed path, Ngmix.get_prior, and self._rng removed; a config that still sets SEED_FROM_POSITION=False raises with a clear message. - RUN_NAME read via getexpanded, so per-chunk run dirs can come from $SP_NGMIX_CHUNK in the committed template. Tests: tests/module/test_ngmix.py 24 passed / 1 pre-existing failure (azgauss needs ngmix>=2.4.1, absent from the runtime image; fails on clean tree too); test_ngmix_weight_validation.py 14 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRBKnHV6oUXEqRpsjqJpex
…ime rewriting (#848 D2) New workflow/config/cfis/ (example/cfis + p3-batch1 deltas, rewritten; example/cfis untouched for the bash flow): - RUN_DATETIME=False committed; every 'last:' INPUT_DIR replaced by the fixed $SP_RUN/output/run_sp_<X>/<runner>/output path of its producer. - NUMBER_LIST=$SP_UNIT_NUM on unit-ID-numbered stages (getlist env-expands; SP_UNIT_NUM carries the dashed form, e.g. -210-282 — rules do the transform). - ngmix template: $NGMIX_ID_MIN/$NGMIX_ID_MAX, SEED_FROM_POSITION=True, per-chunk RUN_NAME=run_sp_tile_ngmix_Ng${SP_NGMIX_CHUNK}u, SAVE_BATCH=250. - merge_sep_cats: N_SPLIT_MAX=$NGMIX_N_CHUNKS; module reads it env-expanded (same getexpanded treatment as the ngmix chunk fields). - make_cat from the nosm variant (what the P3 runs used) — no spread_model dependency. - p3 deltas folded: real INPUT_PATH/RETRIEVE=symlink, exp_Ma external star cat ($SP_RUN/star_cat_exp), PiViVi RUN_DATETIME fix. Rejected: SMP_BATCH_SIZE deltas (superseded by -b {threads}). - tile_Sx from the nomask variant (plain Sx needs an unlisted tile_Ma stage); tile_Uc kept but its run_sp_tile_Gic input path is unverified (Uc is not the production detect variant). All 14 configs pass a strict $VAR-expansion round-trip with the workflow's env stubbed. workflow/config.yaml config_src now points in-repo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRBKnHV6oUXEqRpsjqJpex
…ss check CLI + manifest-driven report (#848 D2/D3) completeness.py gains the 'check <stage> <manifest>' CLI every rule's shell line calls after shapepipe_run: counts products under $SP_RUN against the floor table, writes a byte-stable JSON manifest (per-runner found/expect/ floor/warn, log-scraped failure reasons, no wall-clock), exits nonzero iff a mandatory runner is below floor — manifest always written first. STAGE_DIR maps stages to the committed configs' fixed RUN_NAMEs; the ngmix entry env-expands ${SP_NGMIX_CHUNK} so chunk K checks chunk K's dir. run_report.py rewritten to read manifests instead of disk-scanning: per-tile / per-exposure tables with failure reasons, attrition aggregate (failed units excluded), tile-blocked-by-exposure join, 'not run' for missing manifests. CLI interface unchanged (--run-dir --index --status; --out/--limit added). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5KZRTAZKSBAnpdDhE7yzK
…harded stores, parse-time index, native scattergather (#848 D1–D5) sp_rule.py is deleted; its responsibilities dissolve into the design: config normalization -> the committed S2 configs; log sync -> dead (zero 'last:'); unit furniture -> inline shell (tile_numbers.txt, pseudo-Fe exp_numbers with the ORIGINAL exposure name, star-cat symlinks); count floor -> completeness.py check writing the manifest each rule declares as its only output. SP_UNIT_NUM carries the leading-dash dashed form; the rules do the transform. Stores shard to tiles/<2ch>/<ID>/ and exp/<2ch>/<base>/; build_index accumulates (no DROP TABLE) and is built at parse time of the compute invocation; bin/sp reduces to run (two invocations) + report + cancel. ngmix scatters via the scattergather knob with in-job closed ID ranges (scripts/ngmix_range.py); retries with attempt-scaled mem on transient- exposed rules; protected() dropped; temp(directory()) rides as a scoped secondary output on the vignette store and chunk dirs — the one exception to no-directory-outputs, so native temp() reclaims intra-tile bulk. Deviations proven by implementation: merge_sep_cats INPUT_DIR must stay relative (MergeSep re.sub's the first '1' in the path — absolute sharded paths break chunk discovery); failed jobs need keep-incomplete in the profile or Snakemake deletes the failure manifests sp report exists to read (-> S6). Dry-runs: prepare 13 jobs, compute 141 on the 210/211 quad (19 exposures, 8 ngmix chunks); --set-scatter ngmix=1 -> 113. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5KZRTAZKSBAnpdDhE7yzK
All eleven findings from the fresh-eyes review, repro-verified: keep-incomplete:true (failure manifests are the post-mortem record — Snakemake otherwise deletes exactly what sp report reads); sp run survives a partial prepare (the missing-threshold is the gate, not set -e); the parse-time index build runs only under SP_PHASE=compute, always evaluates the threshold there, and refuses a silent empty run (WorkflowError on zero ready tiles) while passthrough invocations (--unlock, --dag, targets) never build; profile set-threads/set-resources blocks deleted — they silently replaced the rules' attempt-scaled lambdas and fork widths; manifests write only on content change (a byte-identical rewrite churned the whole cone via mtime); per-tile edge refresh in build_index (stale tile→exposure edges are unrepresentable-shrink no more); blame join blocks on failed/missing only, never warn (else all-tiles-blocked-by-all- exposures at production attrition); ngmix range eval no longer swallows script failure; compute DAG gets a regeneration edge to tile_find_exposures; threshold check precedes any durable index write. Manifest 'unit' is the human ID (basename of SP_RUN); build_forest handles a real-dir dst; stale comments (N_SPLIT_MAX expansion, && check form, sp_rule/protected refs) corrected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5KZRTAZKSBAnpdDhE7yzK
…tones (#848 D5, S5) Exposure-store reclamation moves from a standalone script you remember to run into the DAG itself. One `clean_exposure` job per exposure takes every consuming tile's `tile_vignets` manifest as `input:` — vignets is the last stage that reads exposure products — deletes the store, and leaves a `cleaned.json` tombstone as its output. Writer, then readers, then cleaner: DAG-ordered, race-free, rolling mid-run while ngmix still grinds. The consumer set comes from the accumulating index (new EXP_TILES inverse edge), so it spans the campaign, not one invocation. The tombstone / late-append interaction, which is the whole design problem here: * The job deletes the exposure's `manifests/` along with its `output/`. That is load-bearing. The manifests are the exposure rules' DECLARED outputs; had they survived, a tile appended after the clean would find the exposure chain "up to date" and run its vignets against a store that is no longer on disk. With them gone the DAG sees an unbuilt chain and regenerates it — the accepted cost of a late append, expressed as ordinary Snakemake bookkeeping rather than a special case. The tombstone deliberately does NOT stand in for those manifests: it is input to nothing, so it can never mask their absence. * Tiles already finished are not disturbed: Snakemake demands a missing intermediate only when something downstream of it must run. * `params.consumers` carries the consumer set, so growing it makes the tombstone stale under the default `params` rerun-trigger; the clean job then reruns after the new tile's vignets, against the enlarged set. Cleaned once per consumer set, not once per campaign. * Nothing is lost to the report: each manifest's content is copied verbatim into the tombstone before deletion. Scope guard: an exposure is only eligible when every consuming tile is either in this run's scope or already has its vignets on disk. Without that parse-time test, requesting a tombstone for an exposure shared with a later batch would drag that batch's whole tile chain into this DAG through the clean rule's input — scope expansion by cleanup. Ineligible exposures are deferred, never lost: the invocation that finishes their last consumer picks them up. Gated by `clean:` in workflow/config.yaml (default false for the 4-tile P0 run, where the exposure store is exactly what you want to inspect) and by SP_PHASE=compute, so prepare and passthrough parses schedule no deletions. Flipping it on later reclaims retroactively — the missing tombstones schedule exactly the outstanding clean jobs. Invariants held: no temp() on exposure-level outputs; the loud WorkflowError on zero ready tiles; keep-incomplete untouched. Dry-runs on the 210/211 quad (19 exposures): prepare 13; compute 145 with clean off (identical to b149e18 under the same disk state); compute 164 with clean on = 145 + 19 clean_exposure. Prepare and passthrough parses show no clean rules and leave the index untouched. Deviation from the gap-map wording: one wildcarded rule with an index-driven input function, not literally one generated rule object per exposure. The DAG is identical, and at DR6 scale ~20k rule objects is a parse cost with nothing bought. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UxXRKzfu6RUGNLv7ytNsHw
The profile flips from the P0 one-allocation/local-scheduler mode to the PRD's executor: slurm. jobs: 800 = 80% of the queried MaxSubmitPU=1000 on def-mjhudson (sacctmgr, 2026-07-30). default-resources carry mem_mb, runtime, slurm_account: def-mjhudson; keep-incomplete stays (a failed job's manifest is its only post-mortem record). No set-resources/set-threads — rules own their attempt-scaled lambdas. Env pins recorded in the README: snakemake>=9,<10, snakemake-executor-plugin-slurm>=2.7,<3 (installed: 9.23.1 / 2.7.1). group: fusion deferred — it needs rule edits, tracked in the README. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UxXRKzfu6RUGNLv7ytNsHw
…bstones (#848 D5) An adversarial review of the in-DAG clean_exposure design found eight problems. This fixes all eight. The central one is a rerun cascade. THE CASCADE, AND WHAT ACTUALLY CUT IT clean_exposure deletes an exposure's manifests on purpose: that is what makes a tile appended later rebuild the chain instead of running against an empty store. But those manifests are the tile side's inputs, and an exposure is read by ~7-10 tiles. So one rebuilt exposure made every other tile that reads it rerun, and that rerun rebuilt ITS exposures, spreading across the whole exposure-overlap connected component. On fixture t4 (four tiles chained by shared exposures, all complete, all exposures cleaned, one tile's catalogue deleted), asking for the one damaged tile scheduled all four tiles' chains. ancient() alone does NOT fix it, and the measurement says why. Snakemake suppresses the pending-producer propagation only when every shared file is ancient AND EXISTS (dag.py); reclamation has deleted these. Job counts with ancient() alone were identical to before. Three changes cut it, and t4 proves each: * a FINISHED tile (final_cat on disk) drops the exposure manifests that are gone from its input list, and holds the rest through ancient(). It has already extracted everything it will ever read from them, so the propagation has nowhere to go. Only missing manifests are dropped, so a campaign that has cleaned nothing declares exactly the edges it always did. * clean_exposure declares only IN-SCOPE consumers' vignets manifests. An out-of-scope consumer is checked for existence at parse time instead; declaring it pulled that finished tile's whole chain into the DAG. * the profile drops the `input` rerun-trigger. With it on, the conditional edge reads as "set of input files has changed" and reruns the finished tiles anyway — against a store that is gone. Nothing is lost: a changed exposure list still arrives through the find_exposures manifest's mtime, and the ngmix chunk count rides in params. t4, one damaged tile of four: 82 jobs before, 70 with the edge cut but the trigger on, 28 with all three — exactly the damaged tile's chain, its own two exposures, and the clean jobs. Same 28 whether the tile list is the one tile or all four. Appending a tile that shares a cleaned exposure still rebuilds that exposure and reschedules its clean, and touches no other tile (25 jobs). A complete campaign schedules nothing, clean on or off. The cost to know: --forcerun on a tile whose final_cat exists will not rebuild its reclaimed exposures. Delete the final_cat first. THE OTHER SEVEN * sp report read no tombstones, so every cleaned exposure came back "not_run" and blocked the complete tiles whose completion had authorised the deletion. It now reads the absorbed manifests out of cleaned.json, reports the unit as `cleaned` with its warn counts intact, and never blames it for a tile. Fixture t3: 3 exposures "not run" + 2 tiles blocked, before; 3 cleaned + 0 blocked, after. * clean_ignore_tiles: in config.yaml. A permanently-failed tile pinned its ~80 exposures forever; listed there, it leaves the consumer sets. Retrying it later rebuilds those chains from scratch, and the config says so. * the exp_psf benchmark tsv moved out of manifests/, which reclamation deletes. It is the measured-memory feed for mem_mb sizing (D4). * clean_exposure is a localrule. At DR6 scale it was ~20k sbatch submissions to run rmtree. * `--config clean=false` was truthy, so switching reclamation off switched it on. Config booleans now parse strings, and reject what they cannot read. * the tombstone is written, complete and atomically, BEFORE anything is deleted. The crash window now costs disk, never the record. * the false comments in clean_exposure.py and config.yaml are true again. Dry-runs only; no jobs were run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UxXRKzfu6RUGNLv7ytNsHw
…trigger gate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UxXRKzfu6RUGNLv7ytNsHw
…harded, pre-manifest) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UxXRKzfu6RUGNLv7ytNsHw
…on_cpu)
The executor plugin validates against sacctmgr associations, which on nibi carry
the _cpu/_gpu suffix — the bare account that sbatch scripts accept is rejected
at submission ('appears to be invalid'). First-contact finding from the first
real slurm-executor run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UxXRKzfu6RUGNLv7ytNsHw
The SLURM executor re-invokes snakemake inside every job, and that re-invocation inherits SP_PHASE=compute — so every job's parse re-ran build_index.build. Hundreds of concurrent sqlite writers on Lustre produced the 'database is locked' storm that killed the first real run (smk-g3, 2026-07-31): the top-level compute parse died first, and the compute jobs a later invocation submitted kept dying in their own parses. - gate the build on workflow.is_main_process as well as SP_PHASE; job parses now only LOAD the index - timeout=60 on every sqlite3.connect (Snakefile loader, build_index, run_report, build_forest): Lustre lock handoffs outlive the 5 s default even under pure reader traffic - config.yaml: the quad is 186/187 (real files); 210/211's tiles are broken symlinks into anaennis' moved processed_tiles tree, as are ~8.5k of the 10.3k staged tiles Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An sbatch head job starts in the submission directory; snakemake's Snakefile discovery is cwd-relative and found nothing. The launcher already promised cwd-independence — now it delivers it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
P3 fixture measures 7.9 exposures/tile (50 tiles / 181 exposures, sp_p3_batch1-17716364.log). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A manifest was written on failure too, and `keep-incomplete: true` kept it. A head process that died without recording job failures therefore left failed stages looking complete, and a resume scheduled downstream work on top of them (exp_psf on an empty exp_mask output dir, live). completeness.py now forks on the result: success writes `<stage>.json` and removes a stale `<stage>.failed.json`; failure writes `<stage>.failed.json` and removes a stale `<stage>.json`. Exactly one exists, so `<stage>.json` means the stage succeeded. Both keep the write-only-on-change behaviour. The failed manifest is never a declared output, so snakemake neither tracks nor deletes it — which lets the profile drop `keep-incomplete` and get the wanted default back: a failed job's declared output is removed. run_report.py reads either name and takes the status from the manifest body, so manifests written under the old convention still report as failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The PRD assumed mask star catalogues were pre-generated offline. The pre-staged
store covers 1 of the current quad's 18 exposures, and symlinking the whole store
into a unit breaks the mask module outright: the file handler intersects the
image numbers found across a config's INPUT_DIRs, and the store contributes 7240
foreign numbers, so the intersection is empty ("numbers ... do not intersect").
New rule `exp_star_cat`, one per exposure: it runs create_star_cat.py in the
container against the exp_get_images symlink farm (one Vizier cone query over the
focal plane, ~6 s), then builds a REAL per-unit `star_cat_exp/` of 40 per-CCD
symlinks — exactly this exposure's numbers, so the intersection is right by
construction and config_exp_Ma.ini needs no change. exp_mask takes its manifest
as an input.
It is a localrule, so the queries run serially in the head process (an sbatch job
on a compute node, which has internet): CDS is protected at any campaign scale
and the scheduler never sees a six-second job. The catalogue itself is cached
run-independently under `star_cats:`, now the cache root on /scratch, and
create_star_cat.py skips an existing cat, so reruns and later campaigns are free.
unit_pre no longer materialises the star_cat_exp/star_cat_tiles store symlinks.
tile.smk records what a future tile_mask needs on the tile side.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
completeness.py forked on its count verdict alone. A job whose floors were met but whose shapepipe_run exited nonzero therefore wrote the SUCCESS manifest and still exited nonzero. Snakemake then deleted that manifest as a failed job's output, the check had already unlinked any prior failed.json, and the failure left no trace on disk at all. The two statements are now composed. sp_shell passes `--job-rc "$rc"`; a nonzero rc forces the failure branch, records job_rc and a shapepipe_run failure entry that run_report surfaces, and the fork writes failed.json and exits 1 as usual. The rc is recorded only when nonzero, so every existing success manifest stays byte-identical and the mtime rerun-trigger does not churn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four ways the star-catalogue stage lied about its own state, plus one comment that lied about the scheduler. The farm loop wrote into the shared pool. Legacy unit dirs carry star_cat_exp as a symlink into the old shared star-cat pool, `mkdir -p` no-ops on it, and the 40-link loop then followed the link and wrote this exposure's links into the pool (520 stray links found live). The rule now unlinks a symlinked farm before the mkdir. It never rm -rf's a real farm: that farm is its own output, and ln -sfn refreshes it in place. A killed cache write stayed poisonous forever. create_star_cat.py wrote the cache FITS in place, so an interrupted write left a truncated file that the `isfile -> continue` skip trusted for the rest of the campaign — and `test -s` passes on partial bytes. Both branches now write a hidden, PID-tagged temp with a .fits suffix (astropy picks its writer from the extension) and os.replace it into position; a leftover temp is removed in a finally. The dot-prefix keeps the temp out of the `star_cat*` globs. The manifest attested forever. It was the rule's only declared output, so a purged cache (/scratch, 60 days) or a deleted farm left the stage looking complete while exp_mask had nothing to read. The ccd-0 farm link is now declared alongside it. Snakemake's existence test follows symlinks, so a purge leaves that link dangling, the rule reruns, and it re-queries or re-links — the recovery config.yaml already promised. The script was outside the fingerprint. exp_star_cat's params held only paths, so an edit to create_star_cat.py never reran it. STAR_CAT_HASH follows the existing completeness/forest/clean pattern. The hash deliberately does NOT key the cache path: a rerun still takes the skip-if-exists branch, so a semantic change to the query needs the cache cleared by hand. config.yaml now says so. And the localrule comment claimed local execution serialises the queries. It does not: local jobs run in a pool sized by --local-cores (unset here, so the head process's CPU count, 5 as measured), which is the real concurrency cap. The localrule stays — 5-way is ordinary CDS politeness, and the alternative is ~20k sbatch submissions for six seconds of work each. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
clean_exposure targeted only output/ and manifests/, so the star_cat_exp link farm outlived the exp_star_cat manifest that owned it — 40 symlinks belonging to no rule. The farm and the legacy star_cat_tiles link are now targets too. This is consistency, not bytes: the catalogue stays in the run-independent cache, so rebuilding the farm is a relink and no query. Deletion is symlink-safe. Legacy unit dirs carry star_cat_exp as a link into the shared pool, and rmtree would recurse through it and empty that pool for every other exposure in the campaign. A symlink target is unlinked; only a real directory is rmtree'd. Existence is tested as `is_symlink() or exists()`, because exists() follows the link and would skip a dangling legacy one. Separately, astroquery pickled every Vizier response into $HOME/.astropy/cache/astroquery/Vizier at ~2 MB a query (10 queries, 10 pickles, observed live). The workflow already caches the result as a FITS on scratch and skips the query on a hit, so the pickle is a duplicate — and at campaign scale (~25k exposures) it is ~50 GB against a 50 GB home quota. query_vizier now passes cache=False. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Contributor
Author
|
@martinkilbinger this is ready for review, we can validate scientifically with |
…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
…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
martinkilbinger
requested changes
Sep 5, 2026
martinkilbinger
left a comment
martinkilbinger
left a comment
Contributor
There was a problem hiding this comment.
- A few minor modifications required.
- Some open questions to discuss.
- Have @claude make a list of modifications in the config files in workflow/config/cfis compared to the previous examples/cfis, for easy comparison of what's new.
| @@ -137,7 +137,8 @@ def position_seed(ra, dec, ccd): | |||
| difference, inflating ``sigma_m``. Seeding the per-object RNG from sky | |||
| position instead makes the same object draw the same added noise (and the | |||
| same fit guesses) in every branch, so both cancel and the m-bias error | |||
Contributor
There was a problem hiding this comment.
Not introduced in this PR, but nevertheless incorrect: The m-bias does not shrink if noise is deterministic given the object's posion, this confused the Pujol noise cancellation that only works on simulations), with adding noise here for metacal to counter correlated noise. The comment about the m-bias should be removed.
| mask_ratio.append(mask_left.pop(idx)) | ||
| mask_ratio = np.array(mask_ratio) | ||
| mask_left = np.array(mask_left) | ||
| # Deterministic split, seeded from the unit's file number: the |
| ) | ||
| else: | ||
| seed_from_position = False | ||
| if not config.getboolean(module_config_sec, "SEED_FROM_POSITION"): |
Contributor
There was a problem hiding this comment.
Not sure this is necessary. We should just remove those entry from all ngmix config files in the repo.
| REPO_ROOT = Path(__file__).resolve().parents[2] | ||
| SCRIPT = REPO_ROOT / "workflow" / "scripts" / "clean_tile.py" | ||
|
|
||
| TILE = "186.307" |
Contributor
There was a problem hiding this comment.
This unit test works for UNIONS but would fail for other surveys. Do we flag unit tests, such that potential other users only run general tests?
|
|
||
| # Input path where original images are stored. Can be local path or vos url. | ||
| # Single string or list of strings | ||
| INPUT_PATH = /project/def-mjhudson/unions-wl/exposures, /project/def-mjhudson/unions-wl/exposures, /project/def-mjhudson/unions-wl/exposures |
Contributor
There was a problem hiding this comment.
How will this work on systems other than nibi? In the past, I had such paths read via generic sym links, so it would work on any machine. Would it be an option to do this instead of hard-coded directory names?
| runtime = 10, | ||
| slurm_extra = TILE_SLURM_EXTRA | ||
| shell: | ||
| sp_shell("tile_merge_cats", "config_merge_sep_cats.ini") |
Contributor
There was a problem hiding this comment.
Config file name should be changed to be consistent with all others, e.g. config_tile_Ms.ini
|
|
||
| # --- exposure chain --- | ||
| "exp_get_images": {"get_images_runner": dict(expect=3, floor=3)}, | ||
| "exp_split": {"split_exp_runner": dict(expect=121, floor=41)}, |
Contributor
There was a problem hiding this comment.
check why 41. I would say that, given the 3 output files from get_images runner, we should always expect 3 * 40 + 1 (header file) output for split_exp. If not, there are missing HDUs in the exposures.
| "sextractor_runner": dict(expect=120, floor=2), | ||
| "setools_runner": dict(expect=80, floor=2, subpath="rand_split"), | ||
| "psfex_runner": dict(expect=80, floor=2), | ||
| "psfex_interp_runner": dict(expect=40, floor=0, warn=True), |
Contributor
There was a problem hiding this comment.
All the floor numbers should be checked and justified, I am not sure about many of those.
| # (expect=floor: all-or-nothing, every vignette feeds ngmix). | ||
| "vignetmaker_runner_run_2": dict(expect=5, floor=5), | ||
| }, | ||
| "tile_ngmix": {"ngmix_runner": dict(expect=1, floor=1)}, |
Contributor
There was a problem hiding this comment.
Make sure this is for each chunk, not the total output number (which should be n_chunk).
| "tile_detect": ("tile", "run_sp_tile_Sx"), | ||
| "tile_vignets": ("tile", "run_sp_tile_PiViVi"), | ||
| "tile_ngmix": ("tile", "run_sp_tile_ngmix_Ng${SP_NGMIX_CHUNK}u"), | ||
| "tile_merge_cats": ("tile", "run_sp_Ms"), |
Contributor
There was a problem hiding this comment.
would be nice to make the last two names consistent as well by adding the infix _tile.
workflow: update remaining tile run references
workflow: simplify explanatory prose
Contributor
Author
|
Claude on behalf of Cail. Thanks for the review — all suggestions taken. Point by point: ngmix.py m-bias comment — Removed the m-bias claim; the docstring now argues only chunk-independence. (d5c73a1)
UNIONS-specific unit test — Marked Hard-coded paths — Agreed. Data paths now live in one place: That being said, some of the DAG logic — the fused tile group job, node-local vignette staging, Phrasing — Flattened, plus a sweep of the rules, scripts and README for the same: "first-class durable science product" → "keyed by sky position rather than by run"; "the storm that killed the first real run" → "previously caused 'database is locked' errors"; "escape hatch" → "direct command path"; "the whole point of" → "the purpose of". (cbf7e11) mccd — Added
Completeness: the 41, and the floors in general — You're right, and the data agree. Across smk-g6 (127 exposures, 64 tiles, 512 ngmix chunks) every non-warn runner produced exactly ngmix count per chunk — It is per chunk: the check runs inside each
The config comparison you asked for is in the next comment. |
Contributor
Author
|
Claude on behalf of Cail. The config comparison requested in the review: every file in The comparison below is from INI files
|
| Key | Example | Workflow |
|---|---|---|
RUN_DATETIME |
True |
False |
GET_IMAGES_RUNNER.INPUT_DIR |
last:find_exposures_runner |
$SP_RUN/output/run_sp_tile_Fe/find_exposures_runner/output |
GET_IMAGES_RUNNER.INPUT_PATH |
vos:cfis/pitcairn, vos:cfis/weights, vos:cfis/flags |
$SP_INPUT_EXPOSURES, $SP_INPUT_EXPOSURES, $SP_INPUT_EXPOSURES |
GET_IMAGES_RUNNER.RETRIEVE |
vos |
symlink |
The VOS inputs are replaced by workflow-supplied exposure paths, and retrieval changes from VOS download to symlinking.
config_exp_Ma.ini ← example/cfis/config_exp_Ma_onthefly.ini
| Key | Example | Workflow |
|---|---|---|
MASK_RUNNER.INPUT_DIR |
last:split_exp_runner |
$SP_RUN/output/run_sp_exp_Sp/split_exp_runner/output, $SP_RUN/star_cat_exp |
MASK_RUNNER.FILE_PATTERN |
(absent) | image, weight, flag, star_cat |
MASK_RUNNER.FILE_EXT |
(absent) | .fits, .fits, .fits, .fits |
MASK_RUNNER.USE_EXT_STAR |
False |
True |
The mask stage now consumes the workflow’s per-exposure star catalogue ($SP_RUN/star_cat_exp) and declares its four input types explicitly.
config_exp_Sp.ini ← example/cfis/config_exp_Sp.ini
| Key | Example | Workflow |
|---|---|---|
RUN_DATETIME |
True |
False |
FILE.NUMBER_LIST |
(absent) | $SP_UNIT_NUM |
SPLIT_EXP_RUNNER.INPUT_DIR |
last:get_images_runner |
$SP_RUN/output/run_sp_exp_Gie/get_images_runner/output |
The workflow selects the exposure through SP_UNIT_NUM and uses the preceding exposure-stage output explicitly.
config_exp_mccd.ini — new workflow port from example/cfis/config_exp_mccd.ini
| Key | Example | Workflow |
|---|---|---|
FILE.INPUT_DIR |
. |
$SP_RUN/output |
MCCD_INTERP_RUNNER.PSF_MODEL_DIR |
hard-coded /Users/tliaudat/.../fitted_model/ |
$SP_RUN/output |
The port removes the developer-local PSF-model path and points both inputs and fitted models at the workflow run output.
config_exp_psfex.ini ← example/cfis/config_exp_psfex.ini
| Key | Example | Workflow |
|---|---|---|
RUN_NAME |
run_sp_exp_SxSePsfPi |
run_sp_exp_SxSePsf |
SEXTRACTOR_RUNNER.INPUT_DIR |
last:split_exp_runner, last:mask_runner |
$SP_RUN/output/run_sp_exp_Sp/split_exp_runner/output, $SP_RUN/output/run_sp_exp_Ma/mask_runner/output |
SETOOLS_RUNNER.INPUT_DIR |
last:sextractor_runner |
$SP_RUN/output/run_sp_exp_SxSePsf/sextractor_runner/output |
RUN_DATETIME is made explicit as False throughout the file. The input references are changed from run-relative selectors to explicit per-exposure paths.
config_tile_Fe.ini ← example/cfis/config_tile_Fe.ini
| Key | Example | Workflow |
|---|---|---|
RUN_DATETIME |
True |
False |
FILE.NUMBER_LIST |
(absent) | $SP_UNIT_NUM |
FIND_EXPOSURES_RUNNER.INPUT_DIR |
last:get_images_runner |
$SP_RUN/output/run_sp_tile_Git/get_images_runner/output |
The workflow selects the tile through SP_UNIT_NUM and uses the explicit tile image-download output.
config_tile_Git.ini ← example/cfis/config_tile_Git_vos.ini
| Key | Example | Workflow |
|---|---|---|
RUN_DATETIME |
True |
False |
GET_IMAGES_RUNNER.INPUT_PATH |
vos:cfis/tiles_DR6, vos:cfis/tiles_DR6 |
$SP_INPUT_TILES, $SP_INPUT_TILES |
GET_IMAGES_RUNNER.RETRIEVE |
vos |
symlink |
The VOS tile input is replaced by the workflow-supplied tile path and retrieval changes to symlinking.
config_tile_Mc.ini ← example/cfis/config_make_cat_psfex_nosm.ini
| Key | Example | Workflow |
|---|---|---|
MAKE_CAT_RUNNER.INPUT_DIR |
run_sp_tile_Sx:sextractor_runner, last:psfex_interp_runner, last:merge_sep_cats_runner |
$SP_RUN/output/run_sp_tile_Sx/sextractor_runner/output, $NGMIX_VIGNET_DIR/${SP_PSF}_interp_runner/output, $SP_RUN/output/run_sp_tile_Ms/merge_sep_cats_runner/output |
MAKE_CAT_RUNNER.NUMBER_LIST |
(absent) | $SP_UNIT_NUM |
The PSFEx vignette input is taken from the node-local vignette store ($NGMIX_VIGNET_DIR), while the other inputs remain in $SP_RUN.
config_tile_Mh_exp.ini ← example/cfis/config_tile_Mh_exp.ini
| Key | Example | Workflow |
|---|---|---|
FILE.NUMBER_LIST |
(absent) | $SP_UNIT_NUM |
MERGE_HEADERS_RUNNER.INPUT_DIR |
last:find_exposures_runner |
$SP_RUN/output/run_sp_tile_Fe/find_exposures_runner/output |
The workflow selects the tile through SP_UNIT_NUM and uses the explicit exposure-list output.
config_tile_Ms.ini ← example/cfis/config_merge_sep_cats_template.ini
| Key | Example | Workflow |
|---|---|---|
FILE.INPUT_DIR |
./output/run_sp_tile_ngmix_Ng1u/ngmix_runner/output |
$SP_RUN/output/run_sp_tile_ngmix_Ng1u/ngmix_runner/output |
FILE.OUTPUT_DIR |
./output |
$SP_RUN/output |
FILE.NUMBER_LIST |
(absent) | $SP_UNIT_NUM |
MERGE_SEP_CATS_RUNNER.N_SPLIT_MAX |
X |
$NGMIX_N_CHUNKS |
The merge stage uses the workflow output root, selects the tile explicitly, and gets the chunk count from the workflow scatter-gather setting.
config_tile_Ng_template.ini ← example/cfis/config_tile_Ng_template.ini
| Key | Example | Workflow |
|---|---|---|
RUN_NAME |
run_sp_tile_ngmix_NgXu |
run_sp_tile_ngmix_Ng${SP_NGMIX_CHUNK}u |
FILE.NUMBER_LIST |
(absent) | $SP_UNIT_NUM |
NGMIX_RUNNER.INPUT_DIR |
run-relative selectors, including last:X_interp_runner and run_sp_tile_Mh_exp:merge_headers_runner |
$SP_RUN/output/run_sp_tile_Sx/sextractor_runner/output, $NGMIX_VIGNET_DIR/${SP_PSF}_interp_runner/output, $NGMIX_VIGNET_DIR/vignetmaker_runner_run_2/output, $SP_WCS_DIR |
NGMIX_RUNNER.BKG_RMS_VIGNET_PATH |
$SP_RUN/output/run_sp_tile_PiViVi/vignetmaker_runner_run_2/output/background_rms_vignet{file_number_string}.sqlite |
$NGMIX_VIGNET_DIR/vignetmaker_runner_run_2/output/background_rms_vignet{file_number_string}.sqlite |
NGMIX_RUNNER.ID_OBJ_MIN |
X |
$NGMIX_ID_MIN |
NGMIX_RUNNER.ID_OBJ_MAX |
X |
$NGMIX_ID_MAX |
NGMIX_RUNNER.PIXEL_SCALE |
(absent) | 0.186 |
NGMIX_RUNNER.SAVE_BATCH |
(absent) | 250 |
NGMIX_RUNNER.CENTROID_SOURCE |
wcs |
(removed) |
NGMIX_RUNNER.METACAL_PSF |
fitgauss |
(removed) |
The workflow gives each ngmix chunk its own run name and object range, stages the vignette store and WCS directory through $NGMIX_VIGNET_DIR and $SP_WCS_DIR, and bounds batch saving at 250 objects. CENTROID_SOURCE and METACAL_PSF are removed rather than configured here.
config_tile_PiViVi_mccd.ini — new workflow port from example/cfis/config_tile_PiViVi_canfar_uc.ini
| Key | Example | Workflow |
|---|---|---|
MODULE |
psfex_interp_runner, vignetmaker_runner, vignetmaker_runner |
${SP_PSF}_interp_runner, vignetmaker_runner, vignetmaker_runner |
FILE.NUMBER_LIST |
(absent) | $SP_UNIT_NUM |
FILE.OUTPUT_DIR |
$SP_RUN/output |
$SP_VIGNET_OUT |
| module section | [PSFEX_INTERP_RUNNER] |
[MCCD_INTERP_RUNNER] |
MCCD_INTERP_RUNNER.INPUT_DIR |
(absent) | $SP_RUN/output/run_sp_tile_Sx/sextractor_runner/output, $SP_RUN/output/run_sp_tile_Mh_exp/merge_headers_runner/output, $SP_RUN/output/run_sp_tile_Fe/find_exposures_runner/output |
MCCD_INTERP_RUNNER.ME_DOT_PSF_RUNNER |
(absent) | mccd_fit_val_runner |
MCCD_INTERP_RUNNER.ME_DOT_PSF_PATTERN |
(absent) | fitted_model |
MCCD_INTERP_RUNNER.STAR_THRESH, CHI2_THRESH |
22, 2 |
(removed) |
VIGNETMAKER_RUNNER_RUN_1.INPUT_DIR |
run-relative selectors | $SP_RUN/output/run_sp_tile_Sx/sextractor_runner/output, $SP_RUN/output/run_sp_tile_Uz/uncompress_fits_runner/output |
VIGNETMAKER_RUNNER_RUN_2.INPUT_DIR |
run-relative selectors | explicit $SP_RUN paths for Sx, Mh_exp, and Fe outputs |
This is the MCCD variant of the PSF interpolation port; its vignette output is node-local through $SP_VIGNET_OUT, and MCCD models are discovered under $SP_EXP.
config_tile_PiViVi_psfex.ini ← example/cfis/config_tile_PiViVi_canfar_sx.ini
| Key | Example | Workflow |
|---|---|---|
MODULE |
psfex_interp_runner, vignetmaker_runner, vignetmaker_runner, vignetmaker_runner |
${SP_PSF}_interp_runner, vignetmaker_runner, vignetmaker_runner |
FILE.NUMBER_LIST |
(absent) | $SP_UNIT_NUM |
FILE.OUTPUT_DIR |
$SP_RUN/output |
$SP_VIGNET_OUT |
PSFEX_INTERP_RUNNER.INPUT_DIR |
run-relative selectors | explicit $SP_RUN paths for Sx, Mh_exp, and Fe outputs |
VIGNETMAKER_RUNNER_RUN_1.INPUT_DIR |
run-relative selectors | explicit $SP_RUN paths for Sx and Uz outputs |
VIGNETMAKER_RUNNER_RUN_2.INPUT_DIR |
run-relative selectors | explicit $SP_RUN paths for Sx, Mh_exp, and Fe outputs |
VIGNETMAKER_RUNNER_RUN_3 |
present, coadd segmentation vignets | removed |
The PSFEx vignette output is node-local through $SP_VIGNET_OUT; the third segmentation-vignette stage is dropped.
config_tile_Sx.ini ← example/cfis/config_tile_Sx.ini
| Key | Example | Workflow |
|---|---|---|
FILE.NUMBER_LIST |
(absent) | $SP_UNIT_NUM |
SEXTRACTOR_RUNNER.INPUT_DIR |
Git, Uz, Ma, and Mh_exp run-relative inputs | explicit Git, Uz, and Mh_exp $SP_RUN paths; Ma is removed |
SEXTRACTOR_RUNNER.FILE_PATTERN |
CFIS_image, CFIS_weight, pipeline_flag, log_exp_headers |
CFIS_image, CFIS_weight, log_exp_headers |
SEXTRACTOR_RUNNER.FILE_EXT |
.fits, .fits, .fits, .sqlite |
.fits, .fits, .sqlite |
SEXTRACTOR_RUNNER.DOT_PARAM_FILE |
$SP_CONFIG/default.param |
$SP_CONFIG/default_noimaflags.param |
SEXTRACTOR_RUNNER.FLAG_IMAGE |
True |
False |
SEXTRACTOR_RUNNER.CHECKIMAGE |
BACKGROUND, SEGMENTATION |
BACKGROUND |
The tile SExtractor stage no longer consumes pipeline flags, so flag-image input and segmentation check-image output are disabled.
config_tile_Uz.ini ← example/cfis/config_tile_Uz.ini
| Key | Example | Workflow |
|---|---|---|
RUN_DATETIME |
True |
False |
FILE.NUMBER_LIST |
(absent) | $SP_UNIT_NUM |
UNCOMPRESS_FITS_RUNNER.INPUT_DIR |
last:get_images_runner |
$SP_RUN/output/run_sp_tile_Git/get_images_runner/output |
The workflow selects the tile explicitly and uses the Git stage’s absolute output path.
Example files without a counterpart in the workflow directory
These are not represented by a workflow config: config_MsPl_mccd.ini, config_MsPl_psfex.ini, config_MsPl_stars.ini, config_Ms_psfex.ini, config_Ms_psfex_conv.ini, config_Pl_mccd.ini, config_Pl_psfex.ini, config_Pl_psfex_conv.ini, config_Rc.ini, config_exp_Pi.ini, config_get_tiles_vos_headers.ini, config_make_cat_mccd.ini, config_make_cat_psfex.ini, config_save.mask, config_stats.ini, config_tile_Git_cat_vos.ini, config_tile_Ma_onthefly.ini, config_tile_Ng_batch_psfex_uc.ini, config_tile_Ng_template_batch.ini, config_tile_Sx_nomask.ini, config_tile_Uc.ini, config_tile_save.mask, and config_valjoint_Pl_mccd.ini.
The example/cfis/defunct/ directory is also absent from workflow/config/cfis; its dropped configurations are config_Gie_symlink.ini, config_GitFeGie_symlink.ini, config_GitFeGie_vos.ini, config_GitFe_symlink.ini, config_MaMa_onthefly.ini, config_MaMa_save.ini, config_exp_SpMh.ini, config_tile_PiViSmVi.ini, config_tile_PiViSmVi_canfar.ini, config_tile_Sx_exp_mccd.ini, and config_tile_Sx_exp_psfex.ini.
Non-INI files
All non-INI entries in workflow/config/cfis/ are identical to the corresponding entries in example/cfis/: config_onthefly.mask, config_tile_onthefly.mask, default.conv, default.param, default.psfex, default_exp.sex, default_noimaflags.param, default_tile.sex, final_cat.param, mask_default/, and star_selection.setools. They are symlinks to the example entries; no content diff exists.
One question back. The 16 INIs above are edited copies of their example/cfis ancestors, so the two directories now overlap; only the non-INI files are symlinks. Cail has little intuition for which of the example/cfis files (and the bash-era stages with no workflow counterpart) are still needed — if any can be deleted or simplified now that the workflow carries its own set, please say so.
Put more directly: should this PR do the full migration to the workflow (bash layer and its example/cfis set retired), or do you want both systems alive for a while?
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PmtmrAe1k3WtyfPvVBYx2T
Consistent use of the external healsparse masks
Contributor
|
Looks pretty good. One last question: The ngmix ini uses $NGMIX_ID_MIN and $NGMIX_ID_MAX, which I assume to be environment variables. Is this robuts when running on one machine with multiple CPUs, each using different ranges? Your question about example/cfis: I think this directory and content can now be deleted, but please do a last check and flag for me files that are not in the workflow but could potentially be useful. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #848
Closes #873
Closed #844
The PRD in #848 is the design reference; this PR realizes it.
What this replaces: the three-level bash orchestration of real-data runs (
run_job_sp_canfar_v2.0.bash→job_sp_canfar_v2.0.bash→ per-site sbatch wrappers). The bash path is left in place until this workflow is validated and adopted; removing it is a follow-up PR.Shape:
workflow/at top level — Snakefile + three rule files, six stdlib scripts, committed configs with fixed paths, one nibi profile.workflow/bin/spis the entry point:sp run(two snakemake invocations, prepare then compute, joined by a sqlite tile↔exposure index),sp report(standalone, runs mid-flight),sp container(pull/status/sandbox/exec — per-user image cache, resolution sandbox → SIF → configured default),sp cancel.Additional changes not described in #848. Three change scientific behavior for all users, bash path included:
SEED_FROM_POSITION = Falsenow fails loudly instead of running the retired mode.rand_splitis now seeded from the file number (previously unseeded), making the PSF train/validation split reproducible — and different from any previous draw.STAR_THRESH20 → 22 in the twoconfig_tile_PiViVi_canfar_*.ini, aligning with the 80% convention already in the exposure PSF configs.Mechanical enablers and hardening, no output changes: ngmix ID range /
RUN_NAMEread env-expanded;merge_sep_catschunk-path derivation fixed for absolute paths with digits in parent dirs; Vizier responses no longer cached to$HOME(~50 GB at campaign scale); star catalogues written atomically; unit tests for the chunk partition, tile reclamation, and run report.Reviewing: read the PRD's D1–D6 first; each rule file's header comments carry the local why. Run results and DR6 projections are in the benchmark comment below.
🤖 Generated with Claude Code