Coverage nexp masks - #797
Open
martinkilbinger wants to merge 46 commits into
Open
Conversation
# Conflicts: # docs/source/pipeline_canfar.md # pyproject.toml
- Extract _expnum_from_path, _parse_header_to_wcs, _megacam_field_corners as module-level helpers. process_single_header (static, for multiprocessing) and the instance methods now share one implementation. - Remove dead get_wcs_from_header and get_megacam_field (duplicated logic that had already been inlined into process_single_header). - Fix re.search(r'(\d+)\.txt') missing its subject string. The new helper also replaces the hard-coded p[end-6:end] exposure-number extraction in run(), so exposure numbers of any length work. - Declare verbose in params_default so the contract is local instead of relying on cs_util.args.parse_options to inject it. Addresses review feedback on #699.
- Replace hsp_map + 0 with hsp_map.copy() (healsparse supports .copy since at least 1.x; more explicit intent). - Declare verbose in params_default; currently populated only by cs_util.args.parse_options auto-injection. - Fix typo: "Install te cs_util" -> "Install the cs_util". Addresses review feedback on #699.
- coverage_plotter, header_downloader: declare verbose explicitly in params_default. - coverage_plotter: comment acknowledging that FootprintPlotter._regions is a private cs_util attribute; refactor to public accessor when cs_util exposes one. - header_downloader: expand the vos:cfis/pitcairn comment to flag it as UNIONS/CFIS-specific and document override. - build_and_plot_coverage_maps.sh: explain the BUILD_NSIDE=131072 choice (matches UNIONS bit-mask pixel scale, ~0.1"), and note the CoverageMapBuilder default of 2048 for lighter use. Addresses review feedback on #699.
Resolve pyproject.toml conflict: the coverage branch forked before develop's uv/pyproject modernization, so take develop's file as the base and layer in the coverage additions — healsparse + skyproj runtime deps, the plot optional-deps group, plot added to the dev extra, and the six coverage/get-ccds console entry points. All other files auto-merged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HvP7uKkfDU2PvWC7YmbuMW
The coverage feature adds healsparse (map building) and skyproj (plotting) to pyproject. healsparse was already resolved transitively via cs_util; this pins skyproj 2.5.0 and records both as shapepipe's own direct dependencies so 'uv lock --check' passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HvP7uKkfDU2PvWC7YmbuMW
The v>=1.4 path built the full set of exposure single-HDU IDs but never removed the CCDs whose PSF model is missing, so every candidate CCD was reported as valid. Subtract the missing set so the result is the CCDs that actually have a valid PSF, and report the three counts.
Replace the whole-MegaCam-field corner extraction with per-CCD extraction: for each HDU's WCS, build the 4 corners from the CCD pixel bounds (read from pixel_shape, i.e. NAXIS1/NAXIS2) using pixel edges so the quadrilateral covers the full CCD area. Emit one row per CCD keyed on the '<expnum>-<ccd_idx>' ID. Add an optional --ccd_list/-l option: when given, only CCDs in that file (the output of get_ccds_with_psf) are written, so the coverage mask is built from single CCDs with a valid PSF. Resume still keys the done-set on exposure number, now parsed from the CCD-ID first column. Drop the dead _megacam_field_corners whole-field path; keep _parse_header_to_wcs and the multiprocessing structure.
…ards Parse the new per-CCD row format (string CCD ID in column 0, then 8 float corner columns). Add an RA-wrap guard that unwraps polygons straddling the RA=0 seam onto a common branch, and a pole guard that skips polygons reaching |dec| >= 89 deg with a warning (CCD footprints never reach there). Since the CCDs of one exposure do not overlap, accumulating value 1 per CCD polygon still counts exposures-with-valid-PSF per pixel.
Copy each header to a '.part' file in the same directory and os.rename on success. An interrupted transfer can no longer leave a partial file that resume treats as complete; the temp file is removed on failure.
Fix the console-script name typo (get_ccd_with_psfs -> get_ccds_with_psf) so it matches the docs and script. Harden the build/plot script with 'set -euo pipefail' and document the per-CCD chain (step 0 get_ccds_with_psf feeding extract_field_corners --ccd_list). Update the canfar docs: restore the coverage helpers as shipped, align filenames (ccds_with_psf_<version>.txt, exp_ra_dec_<version>.txt), and describe the per-CCD workflow including step 0.
…ownload Rewrite the coverage tests for the per-CCD design: handler missing-CCD subtraction; per-CCD corner extraction and pixel-shape error; --ccd_list filtering end to end; builder parsing of the new row format; RA-wrap unwrap (a CCD straddling RA=0 fills a small patch, not the complement); pole-guard skip; and atomic-download rename behaviour with a mocked copy. Drop the tests that pinned the removed whole-field convention.
Two correctness fixes in the per-CCD extractor: - Image dimensions: header_downloader fetches fpack tile-compressed headers where NAXIS1/NAXIS2 describe the compressed binary table (byte width, row count), so WCS.pixel_shape reports e.g. (8, 4612) instead of the true (2080, 4612). Read ZNAXIS1/ZNAXIS2 when ZIMAGE is set, falling back to NAXIS1/NAXIS2. _parse_header_to_wcs now returns (wcs, shape) pairs since the WCS drops the Z-keywords; _ccd_corners takes the shape explicitly. - Resume: key the done-set on individual CCD IDs, not exposure numbers. A write interrupted mid-exposure previously left a partial exposure that a -r rerun skipped forever (silent undercount); an expanded --ccd_list rerun skipped exposures whose new CCDs were never written. get_done_ccds reads the present CCD IDs and run() drops only those, so partial exposures are completed and new CCDs added, never duplicating a row.
- header_downloader.get_exposures: wrap the text-format loadtxt in np.atleast_1d so a single-line CCD list (0-d array) does not crash on iteration. - coverage_run: update the extract/build runner docstrings from whole-field 'field corner coordinates' to per-CCD language. - pipeline_canfar docs: add a runnable plot_coverage_map example (the SGC region command) covering the flags build_and_plot_coverage_maps.sh uses.
Extend the coverage tests for the confirmed findings: - fpack-compressed header fixture; assert ZNAXIS dims are recovered and corners span the true CCD width, not the 8-byte compressed NAXIS1. - resume run() path: a new exposure is appended without duplicating existing rows, and a partial exposure (interrupted mid-write) is completed rather than skipped. - the nexp contract on pixel values: two overlapping CCDs from different exposures give value 2 in the overlap and 1 outside. - single-row builder file (atleast_1d/2d guards); single-line get_exposures; RA-wrap strengthened to a positional-parity check (seam CCD vs the same CCD shifted +10 deg agree in pixel count); get_ccds_with_psf subtraction pinned against the real get_all_shdus.
Contributor
|
Done — the branch now builds single-CCD-based masks. The chain:
Robustness along the way: extractor resume is keyed on individual CCD IDs (interrupted runs and expanded CCD lists complete cleanly), and header downloads are atomic, so partial files can't poison a resumed run. 57 tests cover the chain — including the nexp value contract (overlapping CCDs from two exposures → 2 in the overlap, 1 outside) — and Update (2026-07-20): — Claude (Fable) on behalf of Cail. |
…v1.6) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqH9wjbJicVmaQdhZ4kBYr
… map The v2 footprint is still growing (>5000 deg2 per the 2026-07 Paris meeting, new Dec<30 region); the newest patch definitions (auxdir/CFIS/tiles_202510) include P10. Default v2.0 to 10 patches and add an explicit -p/--n_patch override so the handler tracks evolving patch layouts without code changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqH9wjbJicVmaQdhZ4kBYr
Martin (PR #812 review): the patch concept is removed in v2.0. For -V v2.0 the handler reads summary/missing_job_32_all.txt and exp_numbers.txt relative to the run root instead of iterating P1..P9; -p restores an explicit P1..P<n> layout when needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqH9wjbJicVmaQdhZ4kBYr
…-less) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqH9wjbJicVmaQdhZ4kBYr
Contributor
|
@martinkilbinger please take a look, should be good now! |
Contributor
|
i think this closes #836? |
This was referenced Jul 16, 2026
Align with the collate_star_cat convention (PR #812): v2.0 (patch-less) is now the default catalogue version. Older, patched layouts are selected explicitly with -V. Help text and the pipeline_canfar.md invocations updated; the v2.0 call drops the flag, v1.x shows -V v1.6. Tests set the version explicitly, so none pinned the old default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqNFhq9uGpDpprYeuGeb8k
WentaoLuo
pushed a commit
to WentaoLuo/shapepipe
that referenced
this pull request
Jul 21, 2026
Lifted from Martin's PR CosmoStat#704 (martinkilbinger:contributors), split off from the unrelated coverage-mask work that now lives in CosmoStat#797. Co-authored-by: Martin Kilbinger <martin.kilbinger@cea.fr> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DJEQwNRApVAQnt53fkyXLZ
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cailmdaley
added a commit
that referenced
this pull request
Aug 31, 2026
Exposures. SExtractor now reads the instrument flag straight from split_exp (`FILE_PATTERN = image, weight, flag` — split_exp already writes an unprefixed flag<num>.fits per CCD), with the run_sp_exp_Ma INPUT_DIR entry gone; mask_query_runner sits between it and setools in both the example and the workflow config, and star_selection.setools cuts on FLAG_EXT == 0 beside every IMAFLAGS_ISO == 0. Same rewiring in config_exp_mccd.ini, which shares that setools file. sextractor_runner's declared input_module follows the real parent. Tiles. config_tile_Sx_nomask.ini becomes THE tile config: tiles have no instrument flag image and no mask module now, so FLAG_IMAGE = False with default_noimaflags.param is the only variant, in example/cfis and in the sims dir. random_cat still needs a pixel mask image, but nothing in the pipeline produces one — its second input is now declared external (the healsparse-native replacement is #797), which is a real capability gap and is flagged as one in the config, the runner and docs/source/random_cat.md rather than papered over. Snakemake. `exp_psf` now depends on `exp_split` directly; `star_catalogue`, `exp_star_cat`, `exp_mask`, `star_cat_cmd`, `in_container` and the STAR_CAT_* helpers are gone, along with `config["star_cats"]` and the two mid-chain localrules. The `exp_short` group goes too, and the docstring says why: it existed to fuse exp_split with exp_mask, and a group of one rule submits exactly the job the ungrouped rule submits. completeness.py trades its exp_mask stage (mask_runner 40/1) for a mask_query_runner row inside exp_psf, floor 2 rather than 40 because setools tolerates the same sparse-CCD attrition either side of it; run_report drops the two stages; clean_exposure stops reclaiming link farms that no longer exist. Verified in the container: all 30 module runners import, every example and workflow config's MODULE list resolves through get_module_runners, and the workflow parses and builds its DAG (`snakemake --lint`, `-n`). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cem4A9vjxA7nkPnyBKrc5W
Contributor
|
@martinkilbinger this is ready for review/sign off! |
cailmdaley
added a commit
that referenced
this pull request
Aug 31, 2026
…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
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.
Summary
Implements creation of coverage healsparse masks encoding the exposures on which PSF information is avialable.
The current code, implemented in
utilitiesdoes the following steps:header_downloader.py)field_corners_extractor.py)coverage_map_builder.py)This can be integrated into a workflow. We also need to implement obtaining the list of valid CCDs with PSF model.
Reviewer Checklist
developbranchCloses #836.