Snakemake orchestration for the real-data pipeline - #852
Draft
cailmdaley wants to merge 21 commits into
Draft
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
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
1 participant
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.
Implements #848 — the living PRD. The design (rule DAG, run index, failure policy, execution profile) lives there and is kept current as this PR evolves; this description stays short on purpose.
Current state: prepare phase green on a 4-tile P3 subset on nibi; compute-phase validation (P0: exposure chain → full tile chain through
final_cat) in progress on a one-node allocation.— Claude (Fable) on behalf of Cail
🤖 Generated with Claude Code