Repository navigation
clean up workflow image sims - #894
Merged
Merged
Conversation
Image-sim m-bias ran through the legacy bash runner and example/cfis_image_sims, a config chain frozen at an older pipeline state (no background vignets, no bkg-rms ngmix, no neighbour/mom-fail flags; its default.* symlinks dangle since e32c4fb). An m measured that way does not calibrate the pipeline that makes the real catalogue. This makes the simulations an input of the same workflow: - input_type: data | image_sims selects $SP_CONFIG. config/cfis_image_sims is an overlay: real files only for the ingestion INIs whose input naming differs (tile Git/Uz/Fe, exposure Gie/Sp -- the sim get_images writes the real-data output names, so every later stage reads identical patterns), everything else symlinked into config/cfis. - psf_model: fake -- the simulations' true PSF. The exposure stage runs only SExtractor (background maps for the vignets); tile_vignets runs fake_interp_runner, which writes galaxy_psf from `psf_dict` under the name the shared configs read (${SP_PSF}_interp_runner). Star-bearing sims can run psfex/mccd unchanged. - unit_pre writes dashed tile numbers for sims (get_images substitutes them verbatim) and exports PSF_DICT when set. A data run's prologue is byte-identical (checked by diffing dry-run shell commands against the base), so no finished data unit is rerun by the params trigger. - bin/sp: SP_PROFILE selects profiles/<name>; SP_RUN_CONFIG replaces (not layers on) workflow/config.yaml and is snapshotted with the code; the venv is optional when snakemake is on PATH. container.py follows both. - profiles/candide: SLURM profile for candide (node-local /tmp bound as /local/scratch for the tile store). Note: completeness.py gains the `fake` tables, and its content hash is a param on every rule, so this lands at a campaign boundary like any completeness edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KNJYtm9z6VeoChxtDLYn4W
…low layout create_final_cat.py -I globbed run_sp_tile_Mc_*, the legacy bash runner's dated run dir; the workflow writes an undated run_sp_tile_Mc. Accept both. The overlay's final_cat.param was a symlink to the real-data list, which names columns make_cat does not write (IMAFLAGS_ISO: the tile SExtractor has no flag image; NGMIX_MOM_FAIL: written as NGMIX_MCAL_TYPES_FAIL) and omits NUMBER and NGMIX_MCAL_TYPES_FAIL, which sp_validation's image-sim extract reads. It is now a real file: the legacy example/cfis_image_sims list plus NGMIX_NEIGHBOUR_FLAG, checked column by column against a smoke-run final_cat. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSjZrhgsGCLAJCLb5Zjmc3
…de excludes The node-local tile store was /local/scratch/sp-<tile>, unique within a campaign but not across concurrent ones. The image-simulation shear branches are concurrent campaigns over the same tile IDs, and on candide /local/scratch is the node's shared /tmp: two branches' tile_shape jobs on n09 shared one store, one tile_vignets wiped it under the other's ngmix, and both failed. The failure can also be silent -- ngmix reading the other branch's vignets. image_sims now prefixes the store name with a hash of the run dir; data campaigns keep the bare name, so their shell commands (a rerun trigger) are unchanged. The tile_shape group's own slurm_extra (the --tmp floor) replaced the profile default and dropped candide's node excludes; its jobs ran on n09 and n17. profiles/candide restates both for the four members via set-resources (slurm_extra only; the rules keep their threads and attempt-scaled mem_mb, checked in a dry run). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSjZrhgsGCLAJCLb5Zjmc3
config_exp_mccd.ini was never ported: it read split_exp and mask_runner outputs through INPUT_MODULE (mask_runner is gone since #847, and with it the pipeline_flag files), had no mask_query, chained setools and MCCD through last:, and left RUN_DATETIME at its default, so the run dir was dated and nothing downstream could find it. psf_model: mccd could not run. It is now the PSFEx chain up to the star selection -- same split-CCD inputs, flag image, mask_query and setools, so both models fit the same stars -- followed by mccd_preprocessing (all CCDs of the exposure into one training and one test catalogue) and mccd_fit_val (fitted_model-<exp>.npy, which the tiles' mccd_interp reads, plus validation_psf-<exp>.fits). merge_starcat and mccd_plots are dropped: they are campaign-level diagnostics, not per-exposure products. The completeness table's MCCD branch had preprocessing at 80 (per CCD; it is 2 per exposure), listed merge_starcat and the plots, and was all warning-only. It now carries the counts this chain produces, mandatory as for PSFEx: an exposure without a model has no PSF on any tile it overlaps. Checked on SKiLLS star sim 1z2z (exposure 2086792): 120/40/80/2 through preprocessing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VSU9pPZjpBjzWorAL88aKQ
MCCD fits one focal-plane model per exposure after the per-CCD stages: 85 min single-threaded for ~2500 stars and 8.3 GB (SKiLLS star sim 1z2z_1, exposure 2086792), and 48 min on 8 BLAS threads -- 1.75x for 8x the cores, which the thread caps forbid anyway. With threads: 8 the fit leaves 7 reserved cores idle for its whole length. The mccd chain now takes 2: the per-CCD stages run 2 wide (minutes), the fit is unchanged, and an exposure reserves about a quarter of the core-hours. PSFEx and fake keep 8. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VSU9pPZjpBjzWorAL88aKQ
Multi-epoch MCCD interpolation checked `"N_EPOCH" not in cat.get_data()`,
a membership test against the FITS record array, which current numpy
rejects ("Cannot compare structured or void to non-void arrays"): every tile
failed in tile_vignets with psf_model: mccd. psfex_interp already tests
`.dtype.names`; so does this now. With it, tile 233.293 of SKiLLS star sim
1z2z_1 interpolates the seven exposures' MCCD models (a few sparse CCDs
lose an epoch, as intended).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VSU9pPZjpBjzWorAL88aKQ
…lags With the ported exposure chain, the mccd fixes and the mccd_interp fix, psf_model: mccd ran SKiLLS star sim 1z2z_1 tile 233.293 end to end: seven exposures' focal-plane models (120/40/80/2/2 each), mccd_interp's galaxy_psf store, vignets, ngmix and a final_cat of 21857 objects. tile_vignets' mccd counts are now mandatory as for psfex, and the README no longer calls mccd "wired but unvalidated". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VSU9pPZjpBjzWorAL88aKQ
… per campaign bin/sp rewrites the source of the `/local/scratch` bind in the launch snapshot's profile (appending one when the profile has none) and creates the directory. tile_local() is untouched: its output is a params rerun trigger, a bind is not, so the key can be set on a resume. candide's 31 GB node /tmp cannot hold dense image-sim tile stores; image-sim campaigns there point this at NFS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qv5MwRiVKa8wnH665dWBia
write_if_changed() used Path.write_text(), which truncates before writing. run_report.py reads every logs/*.json right after compute finishes (the onsuccess hook) and can catch one mid-write as an empty file -- observed during the grid_3 image-sims acceptance test (exp_get_images.json for one exposure came back "Expecting value: line 1 column 1"). Write to a same-directory temp file and os.replace() it into place instead, which is atomic and closes the race.
The committed default carried one person's real nibi paths (tile_list, inputs, container, outputs) baked in. That means `sp run` was not actually the same command on every machine: anyone who forgot to set SP_RUN_CONFIG on a different cluster would either hit a confusing filesystem error somewhere downstream, or silently point at paths that happen to also resolve on their machine but belong to someone else's campaign. config.yaml now carries a REPLACE_ME sentinel for every machine- or campaign-specific path, with a top-of-file explanation that a real run always supplies its own via SP_RUN_CONFIG. The Snakefile checks the keys that have no safe fallback (tile_list, inputs.tiles/exposures, outputs.run_dir/index_db) right after the config file is read and raises one clear, identical error on any cluster if one is still the placeholder, rather than failing confusingly later or not at all. `container:` is left out of that check: a local sandbox or cached SIF is resolved before it, so a placeholder there is often harmless. Also cleans up several stale comments left over from earlier campaign states (smk-g4/g5 narrative, `clean`/`clean_tiles` comments describing "OFF" for a value that had already been changed to `true`) and two "knob" -> "setting" wording fixes.
…in the ini RETRIEVE=symlink was hard-coded in config_tile_Git.ini and config_exp_Gie.ini (both config/cfis and config/cfis_image_sims), which assumes a pre-staged local mirror. That's nibi's layout; candide's real-data ingestion fetches tiles/exposures from VOSpace (RETRIEVE=vos) and has no local mirror. Add `retrieve: symlink|vos` to the run config, validated at parse time, exported as SP_RETRIEVE by unit_pre() and interpolated into the four ini files. input_type=image_sims always forces symlink regardless of the run config's value, since simulation output is generated locally and never lives in VOSpace -- so a `data` run config's `retrieve: vos` reused as a sims campaign's base can't leak into the sims ingestion stages. Verified with a dry run against both input_type=data (retrieve: vos) and the existing image_sims acceptance-test config (still forces symlink) that SP_RETRIEVE renders correctly in each case.
…s a guard REPLACE_ME sentinels made the committed config unrunnable as-is on any cluster, including nibi -- its own actual mainline user. Restore the real, working nibi values (tile_list, inputs, container, outputs) so `sp run` with no other setup does something real and correct there, the same way it always used to. The REPLACE_ME check in the Snakefile stays: it now guards against a future edit that strips these defaults without supplying real ones, rather than gatekeeping every run. Running the committed default from candide now fails honestly on nibi's real, unreachable /project path (FileNotFoundError) rather than on the placeholder check -- correct, since candide cannot see nibi's filesystem and must supply its own paths via SP_RUN_CONFIG regardless.
machine: (the run config) and SP_PROFILE (the env var bin/sp reads to pick profiles/<name>/, default nibi) are two independent switches for the same fact -- which cluster this is -- set in two different places at two different times, with nothing stopping them from disagreeing: export SP_PROFILE=candide but leave a copied run config's `machine: nibi` unedited, and jobs run under candide's SLURM settings against nibi's data paths. Raise a clear WorkflowError at parse time if they don't match, skipped only when `machine:` is genuinely absent (a config predating this key). No code default for `machine:` -- guessing one on its behalf would defeat the point of the check. Also fixes a comment above the REPLACE_ME check left stale by the previous commit: it still called config.yaml "a universal template with no real paths in it" after nibi's real defaults were restored.
…defaults `machine: nibi` selects config.yaml's own real defaults; the same key now also indexes a `machines:` table carrying tile_list, retrieve, inputs, outputs and container for both nibi and candide, so switching `machine:` (kept consistent with SP_PROFILE by the existing check) is enough to point a bare `sp run` at the right cluster's paths. On candide, where the real survey (VOSpace, RETRIEVE=vos) and the SKiLLS simulations (local disk, RETRIEVE=symlink, a different container) need different values, a machine entry nests them under `data:`/ `image_sims:` instead of stating them flat. `$base_dir` in a string is substituted with that machine's own `base_dir:`, so nibi's block states its one common root once instead of four times. A value may be the sentinel `TBD` -- known to be needed, not yet known what it is (candide's real-data ingestion: no local mirror exists there, the real vos: paths and output roots aren't confirmed yet) -- checked at parse time with the same one-clear-error mechanism REPLACE_ME used to provide. Container resolution needed a specific bridge: container.py reads `container:` by regex off the run config FILE, independent of the Snakefile's `config` dict, so a machine-derived container (which only exists after this Python-side merge) would otherwise be invisible to it. Reused SP_CONTAINER (already the documented way to point resolve_image() at a specific image) instead of teaching container.py about `machines:` too -- set only when neither an explicit top-level `container:` nor a user-set SP_CONTAINER already exists, preserving the same override precedence as every other machine default. Also fixes SP_RUN_CONFIG to LAYER on top of config.yaml rather than replacing it wholesale (workflow.configfile(), called conditionally -- it's the plain method the `configfile:` directive itself compiles to, so it works inside an `if`; merges recursively per snakemake.utils.update_config). Necessary for `machines:` to reach the common case at all: every image-sims run and any one-off cluster override already goes through SP_RUN_CONFIG, and under the old wholesale-replace behaviour none of them would ever have seen the table. Safe against the original replace-wholesale concern (an external config silently inheriting an unrelated real campaign's paths) because `machines:` entries are well-scoped defaults gated on `machine:`/`input_type:` and independently checked against SP_PROFILE, not one campaign's actual values reused as another's fallback. Verified: the committed nibi default still fails honestly (on nibi's real, candide-unreachable path) rather than on a placeholder; a machine:/SP_PROFILE mismatch still raises before that; a candide+data run config (all TBD) raises the new clear placeholder error; and the grid_3 image-sims acceptance-test config, with only `machine: candide` added and its explicit retrieve:/container: removed, dry-runs identically to before except for the retrieve: addition already committed separately -- confirming the machines: table now supplies what that config used to have to state itself, without disturbing the already-completed run's resume state.
The machines: table was only understood by the Snakefile. bin/sp read outputs.run_dir/index_db and tile_store_root straight from the config file, so the committed config (outputs now under machines.nibi) broke `sp run` on nibi with a KeyError, and container.py only saw a top-level container: line. scripts/run_config.py now does the whole resolution (config.yaml, SP_RUN_CONFIG merged on top, then machines[machine] [input_type] for anything unset) and all three use it. - container.resolve_image() takes the resolved container explicitly; drops the SP_CONTAINER workaround, which had let a machine default outrank a user's cached SIF, against the documented order. - nibi's machine entry is nested under data: like candide's, so a nibi image_sims run cannot inherit real-data paths. - Unset required keys are reported like TBD, in one error. - README gains a "Run configuration" section; config.yaml, Snakefile and the ini comments are cut down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NzWDtdQbsK1VhvYbDnmTo
config.yaml gains `run: smk-g6`, and nibi's paths use $run instead of the literal campaign name. $base_dir and $run now expand in values set in the run config itself too, not only in machine defaults, so a sims run config can name its output directories by `run:`. A required path left holding an unexpanded $variable is reported like TBD. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NzWDtdQbsK1VhvYbDnmTo
martinkilbinger
force-pushed
the
feat/workflow-image-sims
branch
from
September 17, 2026 05:29
0daf8db to
78cc0ca
Compare
The run config was only settable through an exported variable: invisible in the command, absent from shell history and job scripts, and silently inherited by later commands. `sp <verb> -c my_run.yaml` now takes it on any verb; sp consumes the flag, makes the path absolute and exports SP_RUN_CONFIG itself, which is what the Snakefile, container.py and the snapshot read. An already-set SP_RUN_CONFIG is still honoured, and -c wins. -c is sp's own flag, not snakemake's --cores; cores pass through as --cores/-j. Closes the `sp run --config-file` half of #891 step 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NzWDtdQbsK1VhvYbDnmTo
…ader Typical invocations, the order the run config and config.yaml are read in (and that the machines: table fills the rest), the run_config.py one-liner for inspecting a resolved value, why -c is not snakemake's --cores, and the remaining SP_* variables including SP_PHASE, which sp sets itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NzWDtdQbsK1VhvYbDnmTo
martinkilbinger
marked this pull request as ready for review
September 17, 2026 06:55
cailmdaley
reviewed
Sep 25, 2026
cailmdaley
left a comment
cailmdaley
left a comment
Contributor
There was a problem hiding this comment.
Thanks, Martin: this is a lot of careful work, and it lands the heart of #891, with image sims running as an input mode of the real-data DAG. The config overlay, the shared resolver and the parse-time guards keep the sims-specific part small and hard to misuse. A few small fixes, with details inline:
bin/spswallows-c/--configmeant for the delegated command. This breakssp container exec python -c …(the README's own example) andsp run --config clean=false. Both work on develop.merge_final_catswrites an empty HDF5 and exits 0 whenproducts_diris unset, as in the README example. For this PR, addingproducts_dirto that example is enough. #879 will then switch sims over to itsfinal_cat_merge, so data and sims share one merger.run_template.yamldoesn't resolve as written.
On the notes in the description:
- VOS counts: nothing to change. The
completeness.pycounts are minimums, so VOS's 4 and 6 files pass. A failedvcpcan't slip through either: vos exits viaSystemExit, which theexcept Exceptioninretrieve_onedoesn't catch, soshapepipe_runfails. - nibi: checked there at this commit, in a throwaway worktree.
SP_PROFILE=nibiresolvestile_list,inputs,outputs,index_dbandcontaineridentically to develop.sp run -nwith a two-tile run config builds the prepare DAG and submits nothing.- The layering fits well. Nibi campaigns have so far been launched by editing and committing
config.yamlin a frozen checkout; with this PR, that becomes a run config holding justrun: smk-gN.
— GPT-6 Pro, Claude Opus and Claude Fable, on behalf of Cail
The argv scan for sp's own -c/--config-file took every -c/--config/ --config-file anywhere in argv as the run config, including ones that belong to the command sp delegates to. This broke the README's own `sp container exec python -c ...` (container.py already reads SP_RUN_CONFIG, so its own -c is never sp's run config) and `sp run --config clean=false` (the override flag() in the Snakefile is written for). Stop the scan at `--` and at the command after `container exec`, and drop the `--config` alias so snakemake's own `--config k=v` passes through untouched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without products_dir, merge_final_cats falls back to run_dir and create_final_cat.py finds no matching tile directory under it, so the rule writes an empty hdf5 and exits 0 -- a failure that only surfaces later, in sp_validation, as "No data found". Giving the example the template's <sim>/run + <sim>/product layout avoids that trap; hardening the rule itself is left to #879, which replaces merge_final_cats for sims. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Three problems kept this from resolving: run_config.py only expands $base_dir and $run, so the template's own $my_base_dir and $sim stayed literal and the Snakefile rejected the paths; with no input_type, the run took the data branch (cfis configs, psfex, VOS retrieval) instead of image_sims; and outputs.index_db, a required key, was missing. shapepipe_repo: is also dropped -- nothing reads it. Puts the sim name in run: and uses $run throughout, which run_config.py already expands; the remaining <user-defined directory> placeholders are for hand-editing, same as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Pushed the three fixes from the review so this can go in: — Claude (Opus) on behalf of Cail |
cailmdaley
added a commit
that referenced
this pull request
Sep 26, 2026
…persist-exp-products Conflicts, and how each was resolved: - workflow/config.yaml: #894's layout and terse top-level comments, with its `machines:` table, `run:` and `input_type:`. #879's `persist_exp:` keep-list block and `max_mem_mb:` survive, placed before `clean:` and before `ngmix_chunks:` respectively. #879's `campaign:` commentary, its top-level index_db and config_src commentary go: `run:` names the campaign (next commits), and index_db lives in the machines table. - workflow/bin/sp: #879's wording for why the repo's scripts/ is snapshotted (final_cat_merge loads scripts/python/create_final_cat.py by path) and #879's rsync/cp argument order; both sides copy the same four trees. - workflow/Snakefile: CONFIG_DIR is #894's per-input_type lookup, and #879's SNAPSHOT_JSON definition stays beside it. `rule all` takes #879's persist_targets(), star_cat_targets(), final_cat_targets() in place of #894's merge_targets(); the now-unreferenced merge_final_cats rule is removed in the following commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xYGyn53XoyL83uuKPN7RQ
cailmdaley
added a commit
that referenced
this pull request
Sep 26, 2026
#894's merge_final_cats (image sims only) and #879's final_cat_merge wrote the same product: <products_dir>/final_cat_<name>.hdf5, one dataset per tile under patches/<name>, the CONFIG_DIR final_cat.param columns. Keep final_cat_merge, which reconciles instead of appending and already reads CONFIG_DIR / "final_cat.param", so an image-sims campaign picks up config/cfis_image_sims/final_cat.param with no further wiring. Removed: the merge_final_cats rule, merge_targets(), MERGE_PATCH_DIR, MERGE_ROOT, MERGE_PATCH, MERGED_CAT, CREATE_FINAL_CAT. What changes for an image-sims campaign: the group name comes from the campaign name rather than the basename of products_dir's parent (the two agree under run_template.yaml's <run>/product layout once the campaign is `run:`), and n_tiles_final.txt is no longer written; the tile count is the file's n_tiles attribute. sp_validation's image-sims rules do not read n_tiles_final.txt. Comments that named config/cfis/final_cat.param as the only column list, or merge_final_cats as the source of create_final_cat.py -c's path derivation, now say what holds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xYGyn53XoyL83uuKPN7RQ
cailmdaley
added a commit
that referenced
this pull request
Sep 26, 2026
#879 named the campaign `campaign:`, defaulting to products_dir's basename; #894 introduced `run:`, which `$run` expands to in the run config's paths. One name: CAMPAIGN = config["run"], and the `campaign:` key is gone from the Snakefile, config.yaml, README and the reconcile messages. The merged catalogues are final_cat_<run>.hdf5 and full_starcat_<run>.hdf5. run_config.REQUIRED gains `run`, so a run config whose paths never mention `$run` still refuses at parse time rather than naming the campaign's products after nothing. The shipped config.yaml no longer sets `run:` (it stays as a commented example); the run config does, and run_template.yaml already does. bin/sp: with `run:` unset, outputs.run_dir resolves to a path holding a literal `$run`, and sp created "<...>/$run-state" before the Snakefile could refuse. It now refuses first, naming `run:`, for every verb that needs a campaign; `sp container` and `sp cancel` need none and neither check nor create the state dir. The "config.yaml only" usage line goes. tests/unit/test_run_config.py pins the requirement and the shipped config's reliance on the run config. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xYGyn53XoyL83uuKPN7RQ
cailmdaley
added a commit
that referenced
this pull request
Sep 26, 2026
- Snakefile unit_pre docstring: every prologue line is a params rerun trigger, so lines are added only at a campaign boundary; the image-sims lines are conditional so a data prologue does not carry them. Drops the false "byte-identical" claim (SP_RETRIEVE is exported unconditionally). - create_final_cat.py -c: the patch/group is `run:`, as final_cat_merge names it, and a products_dir not laid out as <root>/<run>/product refuses with a pointer to -i/-P. n_tiles_final.txt is written only when -o is given; the n_tiles attribute is always set. Without -c, -o behaves as before. - hdf5_reconcile.check_sole_group: the path carries `run:`, so a rename makes a new file; the guard is for a file at this path already holding another campaign's group. - config.yaml max_mem_mb: nibi's ceiling; a candide run never reaches it. - bin/sp: header names workflow/scripts/run_config.py; bare `sp`, -h and --help print snakemake's usage instead of refusing on an unset `run:`, and create no state dir. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xYGyn53XoyL83uuKPN7RQ
cailmdaley
added a commit
that referenced
this pull request
Sep 26, 2026
…ed, the MCCD chain runs final_cat.param no longer requests IMAFLAGS_ISO (the tile chain never produced it), and #894's MCCD port reads exp_split's flag files through mask_query_runner, so both [LINT] sentences and their dead anchors go. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
|
@martinkilbinger this is now good for me to merge! |
added 3 commits
September 28, 2026 15:19
…age-sims # Conflicts: # workflow/config/cfis/config_exp_mccd.ini # workflow/rules/exposure.smk # workflow/scripts/completeness.py
…pipe into feat/workflow-image-sims # Conflicts: # workflow/config/cfis/config_exp_mccd.ini # workflow/rules/exposure.smk # workflow/scripts/completeness.py
cailmdaley
added a commit
that referenced
this pull request
Sep 28, 2026
Develop carries #894 (squashed, with Martin's run_config fixpoint / ${name} expansion), #918's MCCD exposure chain, and #859's fourth-moment PSF columns. Resolution: - run_config: develop's variable table (every top-level scalar, ${name}, fixpoint, `run` written back) with this branch's required `run:` and recursive unexpanded-$var refusal. - MCCD chain (config_exp_mccd.ini, completeness.py, exposure.smk comment): develop's #918 wiring; psf_model=mccd stays refused at parse time for persistence. - final_cat_merge replaces develop's merge_final_cats rule; config.yaml keeps `run:` unset (required). - #859 composes with the campaign products: MergeStarCatPSFEX's two-pass _COLUMNS and merge_star_cat.py's COLUMNS gain the six M4/RHO4 columns (22 in both), and cfis/final_cat.param lists the per-epoch HSM_M4_1/M4_2/RHO4_PSF_n slots. - make_cat: taken from the updated feat/make-cat-fixed-epoch-slots. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cailmdaley
added a commit
that referenced
this pull request
Sep 28, 2026
… feat/wire-external-masks Resolved by keeping develop's side everywhere #879's pre-squash content conflicted, then re-applying this branch's own changes (a846e08..dc11dab): - astra.yaml: the mask-cut decision moves into the masking sub-analysis as masking.mask_default_cut, in the #875 format (no Anchor sentence; the make_cat contract tag cites it). masking.sky_mask_application now selects catalogue_columns, since the committed config sets MASK_EXT_PATHS; config_tile_Mc.ini's MASK_EXT_PATHS carries its tag. universes pinned. - cfis_image_sims/config_tile_Mc.ini: rebuilt from develop's cfis copy (spread model gone), still without MASK_EXT_PATHS or per-epoch slots. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cailmdaley
added a commit
that referenced
this pull request
Sep 28, 2026
… feat/retire-versioning Resolved by keeping develop's side where #879's pre-squash content conflicted, then re-applying this branch's own changes (a846e08..bfb7efc). Two by hand: docs/source/pipeline_canfar.md stays retired (develop's one-line path fix goes with it), and the rebuilt tutorial drops the spread-model step that #911 removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Image simulations in the Snakemake workflow, per-machine run config (#891 step 3)
Step 3 of #891: SKiLLS image simulations run through the same workflow as the
real data, instead of a separate orchestrator. Also covers the candide profile
from step 1.
Summary
Input type and PSF model
input_type: data | image_simsandpsf_model: psfex | mccd | fake,validated at parse time.
fakerequiresimage_sims; sims with stars canstill use
psfex/mccd.config/cfis_image_sims/is an overlay ofconfig/cfis/: symlinks foreverything shared, real files only for the five ingestion configs whose
input naming differs,
final_cat.param, and the two fake-PSF configs.psf_model: fake:exp_psfruns SExtractor only;tile_vignetsbuilds thePSF with
fake_interp_runner(from Image simulations workflow #766) frompsf_dict. Completenesstables cover all three PSF models.
Run configuration (
workflow/config.yaml,workflow/scripts/run_config.py)SP_RUN_CONFIGis now merged on top ofconfig.yamlinstead of replacingit, so a run config only states what differs.
machines:table gives, per machine andinput_type, thetile_list,retrieve,inputs,outputsandcontainerused when the run configleaves them unset.
$base_dirand$runexpand in paths;TBDstops therun at parse time.
machine:must matchSP_PROFILE, so a run cannot use one cluster'sSLURM profile with another cluster's paths.
run_config.py) is shared by the Snakefile,bin/spandcontainer.py.retrieve: symlink | vosis a run-config key, exported asSP_RETRIEVE(previously
RETRIEVE = symlinkwas fixed in the ini files). Sims always usesymlink.Other
profiles/candide/.completeness.pywrites manifests and logs atomically;run_report.pycould previously read a log while it was being written.
For review (@cailmdaley)
I tried to make the config settings more flexible across platforms: Now the main workflow/config.yaml contains section for nibi and candide with machine-specific settings and paths; more can be added. The user can add on the command line of sp a user-defined config file that can contain additional settings and/or overwrite the default ones. I added a template in workflows.
machines.nibi.data, withrun: smk-g6. Theresolved values are identical to before (checked by resolving them on
candide), but
sp runitself has not been run on nibi with this branch.SP_RUN_CONFIGnow layers instead of replacing. Please check this fitshow you drive nibi campaigns.
TBD(VOSpace paths, output roots).completeness.pyexpects theget_imagesfile counts of the symlinksetup; the old bash table had different counts for vos.
Acceptance test
One
grid_3tile (262.288,1z2z_grid_3) throughsp runon candide,input_type: image_sims,psf_model: fake:status=success, 5/5 exposuresand all 9 tile stages ok,
final_cat-262.288.fitswith 1621 objects, ~14 minwall-clock.
Testing
grid_3tile on candide.machine/SP_PROFILEmismatch,TBDvalues,unset required keys, unexpanded
$run.tests/unit/test_workflow_run_names.pypasses.sp runon nibi with the committed config.psf_model: mccd.The history includes some back-and-forth on the config design; squash-merging
is probably cleanest.
🤖 Generated with Claude Code
https://claude.ai/code/session_013NzWDtdQbsK1VhvYbDnmTo
Acceptance test
One
grid_3tile (262.288,1z2z_grid_3) throughsp runon candide,input_type: image_sims,psf_model: fake:status=success, 5/5 exposuresand all 9 tile stages ok,
final_cat-262.288.fitswith 1621 objects, ~14 minwall-clock.
Testing
grid_3tile on candide.machine/SP_PROFILEmismatch,TBDvalues,unset required keys, unexpanded
$run.tests/unit/test_workflow_run_names.pypasses.sp runon nibi with the committed config.psf_model: mccd.The history includes some back-and-forth on the config design; squash-merging
is probably cleanest.
🤖 Generated with Claude Code