Skip to content

Coverage nexp masks - #797

Merged
cailmdaley merged 48 commits into
developfrom
coverage
Sep 5, 2026
Merged

Coverage nexp masks#797
cailmdaley merged 48 commits into
developfrom
coverage

Conversation

@martinkilbinger

@martinkilbinger martinkilbinger commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements creation of coverage healsparse masks encoding the exposures on which PSF information is avialable.

The current code, implemented in utilities does the following steps:

  1. Read CCD list and download corresponding FITS exposure images headers (header_downloader.py)
  2. Extract CCD corner coordinates using WCS from header (field_corners_extractor.py)
  3. Create healsparse mask (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

Reviewers should tick the following boxes before approving and merging the PR.

  • The PR targets the develop branch
  • The PR is assigned to the developer
  • The PR has appropriate labels
  • The PR is included in appropriate projects and/or milestones
  • The PR includes a clear description of the proposed changes
  • If the PR addresses an open issue the description includes "closes #"
  • The code and documentation style match the current standards
  • Documentation has been added/updated consistently with the code
  • All CI tests are passing
  • API docs have been built and checked at least once (if relevant)
  • All changed files have been checked and comments provided to the developer
  • All of the reviewer's comments have been satisfactorily addressed by the developer

Closes #836.

martinkilbinger and others added 25 commits January 9, 2026 14:18
# 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.
@martinkilbinger martinkilbinger self-assigned this Jul 9, 2026
@martinkilbinger martinkilbinger added the enhancement New feature or request label Jul 9, 2026
cailmdaley and others added 2 commits July 11, 2026 14:05
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
cailmdaley and others added 3 commits July 16, 2026 03:05
… 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
@cailmdaley

Copy link
Copy Markdown
Contributor

@martinkilbinger please take a look, should be good now!

@cailmdaley

Copy link
Copy Markdown
Contributor

i think this closes #836?

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
@cailmdaley cailmdaley mentioned this pull request Jul 21, 2026
12 tasks
@cailmdaley cailmdaley linked an issue Jul 21, 2026 that may be closed by this pull request
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
@cailmdaley

Copy link
Copy Markdown
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
Comment thread docs/source/pipeline_canfar.md Outdated
> directly under it. Tools that take a catalogue version (e.g.
> `get_ccds_with_psf -V`) follow this convention automatically.

For a `v1.x` run, set the current patch in the shell as

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think v2.0 will still use the previous summary class and outputs. This will be replaced by more robust and efficient bookkeeping of missing CCDs.

elif version == "v1.6":
elif version in ("v1.6", "v2.0"):
# v2.0 (ngmix) covers the same 9-patch footprint as v1.6
n_patch = 9

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the concept of patches is made obsolete in v2.0, this should be replaced by updated handling.

@martinkilbinger

Copy link
Copy Markdown
Contributor Author

I added comments about patches in v2.0: The concept of patches should be removed for v2.0. The script get_ccds_with_psf should be updated or replaced with something to better reflect the new v2.0 processing structure.

…Snakemake stack

Martin (review, 2026-09-04): the v2.0 pipeline is Snakemake-orchestrated,
has no sky patches, and will not use the summary class or its outputs —
so summary/missing_job_32_all.txt and exp_numbers.txt simply won't exist
at a v2.0 run root. The patch-less path added in July was built on files
that are not going to be there.

This PR therefore lands as the v1.x tool. Reverts the v2.0 work
(15401d9, a3aa4ec, ae10176, 46214c7, 1ffcd4b) and restores the pre-July
contract of CcdPsfHandler:

  - accepted versions are v1.3, v1.4, v1.5, v1.6 again; -V v2.0 now
    fails loudly with "Invalid version v2.0"
  - -V default back to v1.6 (was flipped to v2.0 in 1ffcd4b)
  - the n_patch = 0 / patches = ["."] "patch-less" semantics are gone;
    patches are always P1..P<n> from the version map (7/7/8/9)
  - the -p/--n_patch CLI override is dropped: it was introduced in
    a3aa4ec purely to let the evolving v2.0 footprint be overridden, and
    the v1.x patch counts are fixed history

Docs lose the "Patch convention by catalogue version" note and the
patch-less get_ccds_with_psf invocation; the two v2.0 tests
(test_v2_is_patchless, test_n_patch_option_overrides_version_default)
are removed. The v1.x version->patch-count parametrisation and the
invalid-version test stay.

Everything else from the July rework is untouched: per-CCD extraction,
RA-wrap and pole guards, atomic downloads, resume, ZNAXIS dims.

v2.0 coverage will be a separate PR against the Snakemake stack.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@cailmdaley

Copy link
Copy Markdown
Contributor

Thanks Martin, you're right about v2.0: those summary files won't exist. I removed the v2.0 references so this lands as the v1.x tool. For v2.0 we'll open a separate PR on the Snakemake stack once this is in develop: valid-PSF CCDs from the persisted PSF products (#879), corners from the split-exposure headers, reusing the mask builder from here. Does that order work for you?

(Also re-locked cs_util to pick up CosmoStat/cs_util#82FootprintPlotter.plot_area had gone missing in April, so plot_coverage_map was raising AttributeError on develop.)

— Claude on behalf of Cail

@cailmdaley
cailmdaley merged commit 21570d1 into develop Sep 5, 2026
3 checks passed
@cailmdaley
cailmdaley deleted the coverage branch September 5, 2026 13:06
@cailmdaley

Copy link
Copy Markdown
Contributor

Given that Martin's feedback only touched on the v2 question, which deserves its own PR, I'm merging this so it's not blocking downstream work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build ShapePipe per-CCD failure mask [NEW FEATURE] Create coverage masks for UNIONS

2 participants