From 83ea80f63f65361caef4b87a74fb984eecd53f3a Mon Sep 17 00:00:00 2001 From: Cail Daley Date: Mon, 5 Oct 2026 09:43:37 -0400 Subject: [PATCH] tile_vignets and tile_ngmix keep their module logs (campaign boundary) A chunk's ShapePipe run dir is temp(), removed once tile_merge_cats has read it, and tile_vignets' run dir lives in the node-local store, so both runs' logs went with their dirs: the ngmix process logs that carry each chunk's `epoch cuts:` line and every per-object "ngmix failed" message were gone after any campaign, classic or single-allocation. Each rule now copies its run's logs/ files to $SP_RUN/logs/modules// before it exits, whatever its rc, without changing that rc. The shell change moves the params pin in tile_vignets and tile_ngmix, so land it at a campaign boundary. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01D8wdTpGpBtyK6uHwU7qWQW --- tests/workflow/README.md | 1 + tests/workflow/params_pin.json | 6 +++--- tests/workflow/test_dag.py | 23 +++++++++++++++++++++++ workflow/rules/tile.smk | 26 ++++++++++++++++++++++++-- 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/tests/workflow/README.md b/tests/workflow/README.md index b26309227..509670be3 100644 --- a/tests/workflow/README.md +++ b/tests/workflow/README.md @@ -77,6 +77,7 @@ Apply mutations only to a disposable checkout, run the named test without `--upd | `test_final_cat_merge_reads_every_ready_tile` | Drop one ready tile; append an out-of-scope tile. | | `test_products_use_products_dir_and_run_name` | Rename either merged catalogue or the persist manifest; route products to scratch; derive `CAMPAIGN` from the products directory's basename. | | `test_tile_store_is_unique_per_campaign` | Make `LOCAL_TAG` conditional on `image_sims` (or constant); give one tile_shape member a different store path. | +| `test_module_logs_outlive_their_run_dirs` | Drop `post=` from tile_vignets or tile_ngmix; point the copy at `$SP_LOCAL` or `$SP_RUN/output`; move it after `exit $rc`. | | `test_missing_run_fails_during_parse` | Remove `run` from `run_config.REQUIRED`; literal paths must still receive the required-key diagnostic, not a later `KeyError`. | | `test_unit_pre_changes_at_campaign_boundary` | Append a line to `unit_pre`; change one rule's `params.pre`; change one shell; change a rendered thread count; make `LOCAL_TAG` empty for data. | | `test_params_pin_ignores_fixture_root` | Remove fixture-root or run-dir-hash normalization. | diff --git a/tests/workflow/params_pin.json b/tests/workflow/params_pin.json index 7b839ba7e..12b5ef8e9 100644 --- a/tests/workflow/params_pin.json +++ b/tests/workflow/params_pin.json @@ -19,12 +19,12 @@ "tile_make_cat": "ea546ec59bcd13c0f8c9ee2c7fa5dde4773975277eed8f46ab63e6d124db0535", "tile_merge_cats": "ff21216ea804dccc2d2c290d2b2499d5d05f0c34c0a56993c233f43fe3c06bdb", "tile_merge_headers": "7a344849d62936e2f5598dc8731a2c4947eff2c4c7218b1a731dd2a9577e7111", - "tile_ngmix": "5192e65a72b3b6186b29d7bbecfe48751c7bc9ec68a61083ef4092db9f86f02f", + "tile_ngmix": "153fd4cc8328aa2fe3ad3defa27318038aa95334f4e9f290ffaf6ebc293beb97", "tile_uncompress": "1e2b01acbf9708e0371070fb01c5b9568d7efb5f89d1835fc6bf91e2c8b60cb3", - "tile_vignets": "ab52bf2c6ede04915c77f30a44be0cf707c4609ddf8ee768bfaf598cfff31481" + "tile_vignets": "fed959447056258adaf777bee516c0f3e384d9ffaa4cb7a86eb93b4db89580be" }, "schema": 1, - "sha256": "1c792868397e4a2f1144d68d153c4a0a23e9ffd07ecdb95b98896d1b990c8ce1", + "sha256": "7b5b487680924a16ffb1e1df34a78318fe94e2c8dcaf6b45a87c9739bb94abaf", "unit_pre": { "exp_get_images": "8dec850af212879f225fcf27a5f1281e1a075264c7b97d38c2214395d360168c", "exp_psf": "f2358ddf7385918dc5033d10b37f6dc97a15d02b071a3ea0a4619a5f7e6f5bec", diff --git a/tests/workflow/test_dag.py b/tests/workflow/test_dag.py index b23493ecc..35c06688e 100644 --- a/tests/workflow/test_dag.py +++ b/tests/workflow/test_dag.py @@ -145,6 +145,29 @@ def test_tile_store_is_unique_per_campaign(campaign, tmp_path, resolve_dag): assert all(first[tile] != second[tile] for tile in first), (first, second) +def test_module_logs_outlive_their_run_dirs(campaign, dag): + """tile_vignets (node-local) and every ngmix chunk (temp()) copy their + module logs into the tile's logs/modules/ on the shared root, after + shapepipe_run and before the rule exits, whatever its rc.""" + sources = {"tile_vignets": lambda job: '"$NGMIX_VIGNET_DIR"', + "tile_ngmix": lambda job: f'"{job.output.chunkdir}"'} + for rule, source in sources.items(): + for job in dag.jobs_for(rule): + name = ("run_sp_tile_PiViVi" if rule == "tile_vignets" else + f"run_sp_tile_ngmix_Ng{job.wildcards.chunk}u") + lines = job.shellcmd.splitlines() + keep = [i for i, line in enumerate(lines) + if f'"$SP_RUN/logs/modules/{name}"' in line] + assert len(keep) == 1, (rule, job.wildcards_dict) + line = lines[keep[0]] + assert f"cd {source(job)}" in line + assert '-path "*/logs/*"' in line and line.rstrip().endswith(">&2") + run = next(i for i, l in enumerate(lines) + if l.startswith("shapepipe_run ")) + assert run < keep[0] < len(lines) - 1 + assert lines[-1] == "exit $rc" + + def test_missing_run_fails_during_parse(campaign, resolve_dag): """Explicit paths cannot bypass the required campaign name diagnostic.""" campaign.omit_run() diff --git a/workflow/rules/tile.smk b/workflow/rules/tile.smk index f607e3617..15a7bd3cc 100644 --- a/workflow/rules/tile.smk +++ b/workflow/rules/tile.smk @@ -346,6 +346,25 @@ fi +# THE MODULE LOGS OUTLIVE THE RUN DIRS THAT HOLD THEM. A chunk's ShapePipe run +# dir is temp() (tile_merge_cats is its last reader) and tile_vignets' lives in +# the node-local store, so both kinds of log went with their dirs: the +# run's own logs/ and each module's logs/process-*.log, where ngmix writes its +# per-chunk `epoch cuts:` line and every per-object "ngmix failed" message. +# The rule copies them to $SP_RUN/logs/modules// (the tile dir on the +# shared root) before it exits, whatever its rc, so a failed chunk keeps its +# evidence too. A few KB per chunk; clean_tile reclaims them with the rest of +# logs/. The copy never changes the rule's exit status. +def keep_module_logs(run_root, run_name): + dest = f'"$SP_RUN/logs/modules/{run_name}"' + return ( + f'rm -rf {dest} && mkdir -p {dest} && ' + f'(cd "{run_root}" && find . -path "*/logs/*" -type f ' + f'-exec cp -p --parents -t {dest} {{{{}}}} +) || ' + f'echo "could not keep the module logs of {run_root}" >&2\n' + ) + + def tile_exp(wc): return tile_exposures(wc.tile) @@ -612,7 +631,8 @@ rule tile_vignets: # The completeness check is pointed at the NODE-LOCAL run root; see # sp_shell's check_args for what the two flags do. sp_shell("tile_vignets", f"config_tile_PiViVi_{PSF_MODEL}.ini", - check_args=' --run-dir "$SP_LOCAL" --unit {wildcards.tile}') + check_args=' --run-dir "$SP_LOCAL" --unit {wildcards.tile}', + post=keep_module_logs("$NGMIX_VIGNET_DIR", "run_sp_tile_PiViVi")) # ngmix shape measurement — N chunks per tile (D4). Each chunk LOOKS UP its own # CLOSED catalogue-row range in the file tile_vignets materialised at the top of this @@ -828,7 +848,9 @@ rule tile_ngmix: runtime = lambda wc, attempt: 120 * attempt, slurm_extra = TILE_SLURM_EXTRA shell: - sp_shell("tile_ngmix", "config_tile_Ng_template.ini") + sp_shell("tile_ngmix", "config_tile_Ng_template.ini", + post=keep_module_logs("{output.chunkdir}", + "run_sp_tile_ngmix_Ng{wildcards.chunk}u")) def ngmix_manifests(wc):