Emit the instrument flags as a healsparse defect map (exp_defect_map, defect_map_merge) - #887
Open
cailmdaley wants to merge 7 commits into
Open
Emit the instrument flags as a healsparse defect map (exp_defect_map, defect_map_merge)#887cailmdaley wants to merge 7 commits into
cailmdaley wants to merge 7 commits into
Conversation
…878) The exposures' instrument flag images — bad columns, saturated pixels, bleed trails — are the one masking input this campaign has that never leaves the pixel domain: exp_split splits them per CCD, SExtractor reads them as IMAFLAGS_ISO, and that is the end of it. The survey footprint is built from the CCD corner WCS in the headers, so it cannot subtract them and would silently include defective pixels. The lost area is percent-level, but it carries exactly the thin small-scale geometry an accurate window function needs, and only ShapePipe ever opens these files. Two rules, both in exposure.smk. exp_defect_map rasterizes ONE exposure's flag splits into a boolean healsparse fragment on the persistent root — nside_sparse 131072 over nside_coverage 128, bit-packed, True = masked, the same form as every map in the UNIONS ladder, so it drops into that ladder without a resolution change. It hangs off exp_split rather than exp_psf, so re-rasterizing at a different fidelity never touches the four-hour PSF chain, and clean_exposure takes its manifest as an input, so reclamation cannot overtake the copy. The WCS comes from the image split beside each flag file (headers only, never pixels): the flag mosaic's HDUs carry no WCS at all — checked on a real exposure — and headers-<num>.npy is a pickled array of astropy WCS instances a second consumer should not inherit. Rasterization samples each flagged CCD pixel across its full extent, corners included, and is CONSERVATIVE: a healpix pixel is masked if any part of the flagged region touches it, because the centre-based alternative erases a one-pixel bad column, which is the geometry #878 exists to keep. `oversample` rides on params with the measured convergence table behind its default of 3. defect_map_merge unions the campaign's fragments into <products_dir>/defect_map_<campaign>.hsp. It RECONCILES like final_cat_merge — a new exposure is OR-ed in on the spot, an exposure that left the campaign or a fragment that changed forces a rebuild (a union cannot be un-OR-ed), a no-op leaves the file untouched — against a sidecar that records which exposures are already in it, declared as a second output because the two are only meaningful together. Memory is flat in the exposure count: each fragment is read, reduced to its pixel ids and dropped, so the job holds one accumulator (the campaign's footprint) and one 2 MB fragment whether the campaign is 127 exposures or 20k. The map is a campaign PRODUCT, not an input: nothing here reads it back. Measured on 2079612p inside the container, 40 CCDs, 4.4% of pixels flagged: 37 s and 0.62 GB peak RSS at oversample 3, a 2.0 MB fragment of 359858 healpix pixels over 13 coverage pixels (95 s / 1.29 GB / 360840 pixels at oversample 5, so the default is within 0.3% of it at a third of the cost). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
…he ladder config_tile_Mc.ini does NOT gain a MASK_EXT_PATHS entry, and the header now says why: every path in that ladder exists before the run starts, and the defect map exists only after it, so wiring it would name a file the first run of a fresh campaign cannot have and every tile would fail on a missing map. What the header gains instead is the recipe — where the map lands, copying or symlinking it into the inputs.masks root, the one MASK_EXT_PATHS entry and the matching final_cat.param line — plus the two things to know before doing it: the map is the producing campaign's own footprint, so a catalogue built from a different tile list reads False (unknown, not clean) outside it, and the rasterization is conservative, widening a one-pixel bad column to the 1.61" healpix resolution. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
Three defects in the two scripts, found reviewing them. A CCD WHOSE FLAG SPLIT IS MISSING was silently dropped. ccd_files globbed flag-*.fits and rasterized whatever came back, so an incompletely materialised split dir — an age-based scratch purge deleting files one at a time, a store copied or restored half way, a truncated rsync — produced a fragment covering half the exposure, written with "status": "complete", and nothing downstream able to notice. Reproduced on a fixture: remove one flag split of two and the script exited 0 with n_ccds 1. The guard it had was on the opposite, unreachable direction (a flag without its image), which split_exp cannot produce. The loop is now driven by the EXPECTED CCD count, which comes in on params from config_exp_Sp.ini's own N_HDU, and a short split dir is a hard error. A FULLY FLAGGED CCD blew the rule's memory budget. A MegaCam exposure routinely carries a dead or saturated chip: 2048 x 4612 = 9.4M flagged pixels, 85M samples at oversample 3, held in one shot several GB of coordinate arrays and astropy temporaries against a 2 GB request — so the job OOMed on all three attempts, the exposure never got a fragment, and because clean_exposure waits on that manifest it could never be reclaimed either. The 0.62 GB measured on a 4.1%-flagged exposure said nothing about that case. The flagged-pixel loop is now batched at CHUNK source pixels and np.unique-accumulated, so peak RSS is set by the batch and not by how bad the CCD is. Measured on exactly that case, a fully flagged chip against 2079612p's real WCS: 0.74 GB and 18.3 s at 500k, 1.13 GB and 18.9 s at 1M — time is flat in the batch size and memory is linear in it, so 500k is free. The real exposure is unchanged bit for bit (359858 healpix pixels, 2.0 MB) at 34 s. THE SIDECAR WENT STALE ON A NO-OP. main() returned on an empty plan without touching it, but two of its fields describe the CAMPAIGN and not the map: add tiles whose exposures all lack fragments and campaign_exposures and exposures_without_fragment are wrong on disk while the union is untouched and correct. That is exactly the record the docstring promises is "recorded rather than merely printed". The record is now built apart from writing the map and rewritten alone when it differs; write_stable keeps the map's mtime where it is. Also: the stamp is IMPORTED from hdf5_reconcile rather than re-derived, so the two halves of a campaign's reconciliation answer "did this source change" the same way; and the docstring now says plainly, in hdf5_reconcile's own terms, that the map's CONTENT is a function of the input set and its BYTES are not (measured: 1,586,880 B rebuilt vs 1,589,760 B appended, identical valid_pixels). tests/unit/test_defect_map_reconcile.py pins the four reconcile branches, the sidecar refresh and the nside mismatch — fragments made with make_empty, since the merge half never opens a FITS image or a WCS. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
…or 7 GB Four defects in the DAG wiring and the sizing. A RERUN TRIGGER RE-AVALANCHED THE CAMPAIGN. defect_map_inputs() named the manifest of any exposure that had one, reclaimed or not, but exp_defect_map's input is exp_split's manifest and that went with the scratch store. So any trigger on the rule — and params is live, carrying the script hash, the nside and the oversampling this file advertises as a minute's work to change — would schedule exp_get_images and exp_split from VOS for every reclaimed exposure: a four-hour chain each, campaign-wide, on a one-character edit. It now mirrors star_cat_inputs(): a live exposure through its manifest, a reclaimed one through its FRAGMENT, which is not a declared output of any rule and is therefore a true leaf. That also gives prod_exp_fragment() its only caller, so the fragment's location is spelled once here and once in merge_defect_map.fragment_path() rather than twice with nothing joining them. Verified on a two-exposure compute fixture: with 2079612 reclaimed and holding a fragment, requesting the map builds 4 jobs (one exp chain, the live exposure's) where both-live builds 7. THE FIRST MERGE OF A LARGE CAMPAIGN ASKED FOR ~52 GB. Before there is a sidecar the coverage-pixel count was estimated as 13 per exposure capped at the FULL SKY, and 12 * 128^2 = 196608 is reached at ~15k exposures — so a DR6-scale campaign estimated the whole sky, 25.8 GB of accumulator and ~52 GB of request (~104 GB on the retry) for a job this file's own comment measures at ~3 GB. Exposures overlap almost completely; the prior has to be a FOOTPRINT, so it is now capped at the ~23k coverage pixels the UNIONS ugriz maps measure over the same sky. A small campaign is still sized on its own exposures, where the per-exposure figure is the honest one: the two-exposure fixture asks for 806 MB. THE MERGE'S RUNTIME REACHED 11.4 H WITH NO RETRIES AND NO PARTIAL STATE, so its mem_mb/runtime attempt scaling was dead code and one walltime overrun eleven hours in lost the whole thing. retries: 1 is declared, the formula is corrected to the ~1 s per exposure actually measured, and it is capped below the walltime Alliance policy lets a job run without checkpointing — with the comment saying that a campaign needing longer needs a resumable accumulator, not a bigger number. SILENCE WAS THE ANSWER when no exposure can contribute. A campaign whose exposures were all reclaimed by a workflow predating this rule builds no defect job at all, and the operator saw nothing rather than "no map is possible here". It now says so once at parse time, like the missing-index note. Also: a comment pointed at DEFECT_COV_BYTES, which does not exist. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
…n one The README and config.yaml carried the pre-batching numbers and an inode estimate a third of the real cost. The DR6 inode figure was ~20k against a ~1M group quota; it is ~60k, because each exposure gets a defect/ DIRECTORY, the fragment in it and a manifest under manifests/ — three inodes, not one. The fragment was also described as sitting "beside its PSF tar", which is not the layout; the path is spelled out. The timing is the re-measured 34 s (2079612p, 40 CCDs, 4.1% of pixels flagged, oversample 3, inside the campaign container), and both files now carry the worst-case memory bound the batching exists for — a fully flagged chip at 0.74 GB — rather than only the average exposure's 0.62 GB. The README also records what the reconcile does and does not promise (content, not bytes) and points at the new unit test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
… run clean_exposure's exp_defect_map edge was unconditional, which reopened the avalanche defect_map_inputs() is written to avoid. An exposure whose scratch store went to the 60-day /scratch purge, or to a `clean: false` run, has NO tombstone — exp_store_reclaimed()'s docstring names that case — so clean_targets() still asks for its tombstone, the missing exp_defect_map manifest is scheduled, and its own input (exp_split's manifest) went with the store: exp_get_images and exp_split from VOS, four hours per exposure, campaign-wide, on the first run of this branch. Reproduced on a fixture campaign (tile 000.000, exposures 2079612 live and 2079613 purged-without-tombstone, tile_vignets present): a dry run of 2079613's tombstone scheduled 13 jobs including exp_get_images and exp_split; with the edge made conditional it schedules one, clean_exposure. The live exposure still gets exp_defect_map ordered before its clean, and a reclaimed exposure that already has a fragment depends on the fragment, a leaf that builds nothing. ccd_files' short-split error now says what to do about the store it pins: a damaged split dir is still a hard error rather than a half-footprint fragment marked complete, but the operator is told that deleting the scratch exp_split manifest is what rebuilds the split and unblocks the clean. defect_map_merge's mem_mb also goes through capped_mem(), as star_cat_merge and final_cat_merge already do: defect_map_cov_bytes() scales as nside^2 and defect_map.nside is an advertised knob, so one ladder change turns the request into a job SLURM never schedules and snakemake never diagnoses. Verified on the fixture with max_mem_mb=700: "defect_map_merge: sized at 806 MB, capped to max_mem_mb=700" at parse time, mem_mb=700 on the job. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
A rebuild set pixels into a fresh accumulator fragment by fragment, so every fragment that touched an unseen coverage pixel made healsparse grow — and copy — the sparse array. union_coverage() now reads each fragment's COVERAGE TABLE first (HealSparseCoverage.read, kilobytes, never the sparse array) and seeds make_empty(cov_pixels=...) with the union, so the final array is allocated once. MEASURED rather than assumed, on synthetic fragments at the campaign's own resolution (nside 131072 / coverage 128), 400 fragments over 5131 coverage pixels, a 656 MB accumulator: accumulation 8.4 s unseeded, 7.1 s seeded, with a 0.7 s coverage pre-pass. The reallocations are ~16% of the accumulation, not the dominant term a naive reading predicts — healsparse grows the array in blocks — so the review's tens-of-TB estimate does not hold, but the pre-pass is linear where the copying is not, and the win grows with the accumulator. Only the rebuild path seeds: an append starts from the map on disk, whose coverage is already most of the footprint. A fragment at the wrong coverage resolution abandons the seed so accumulate()'s named error still stands; the existing nside-mismatch test pins that. tests/unit/test_defect_map_reconcile.py passes (7/7, run in the container), and an end-to-end fixture merge reproduces rebuild == OR of fragments, no-op untouched, append, and removal-rebuild. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar
This was referenced Sep 10, 2026
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.
Stacked on #886. Closes #878.
Two rules in
exposure.smk.exp_defect_maprasterizes one exposure's per-CCD instrument flags (bad columns, saturated pixels, bleed trails) into a boolean healsparse fragment at nside 131072 / coverage 128, bit-packed — the mask ladder's own convention.defect_map_mergeunions the campaign's fragments intodefect_map_<campaign>.hsponproducts_dir, reconciling likefinal_cat_merge: new fragments OR in on the spot, a changed or removed fragment forces a rebuild, a no-op leaves the file untouched.Two things worth knowing before reading the code. The flag mosaic's HDUs carry no WCS at all, so the rasterizer takes it from the image split's header instead. And the rasterization is conservative — a healpix pixel (1.6″) is masked if any part of a flagged CCD pixel (0.19″) touches it, since the centre-based alternative erases a one-pixel bad column — which widens a bad column's area by roughly 9×. That's the intended tradeoff, not a bug.
It joins the
MASK_EXT_PATHSladder as a documented recipe, not a wired config entry: the ladder names maps that exist before a run starts, and this one exists only after.config_tile_Mc.ini's header carries the recipe for adding it once a campaign has produced one.Cost, measured on one real exposure inside the container: 34 s, 0.62 GB peak RSS, 2.0 MB fragment at oversample 3. At DR6 scale (~20k exposures): ~40 GB and ~60k inodes.
Verified: fixture campaigns and a unit test suite (
test_defect_map_reconcile.py) covering the four reconcile branches, sidecar refresh, and nside mismatch; a dry run confirming the DAG wiring doesn't reopen the exposure-reclamation avalanche. Not verified: merge memory/runtime at DR6 scale (extrapolated), and there is no DAG-level automated test since snakemake isn't in the test venv. First real run: the next 64-tile campaign.🤖 Generated with Claude Code
https://claude.ai/code/session_01QbnPCyzuDNTgkg715pHhar