Skip to content

Image-simulation m-bias: extracted core + snakemake orchestration#225

Open
cailmdaley wants to merge 16 commits into
developfrom
feature/image-sims-orchestration
Open

Image-simulation m-bias: extracted core + snakemake orchestration#225
cailmdaley wants to merge 16 commits into
developfrom
feature/image-sims-orchestration

Conversation

@cailmdaley

@cailmdaley cailmdaley commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Closes #224
Closes #226

Image-simulation m-bias: the sp_validation half

This is the sp_validation-side half of the image-simulation validation story (umbrella: UNIONS-WL/MultiBand_ImSim#1). It pairs with the ShapePipe-side module enablement in CosmoStat/shapepipe#766: that PR makes ShapePipe able to run on the SKiLLS sims; this one makes the whole chain runnable end-to-end and produces the shear-bias result, on sp_validation's native snakemake/container framework.

The split follows the dependency arrow — ShapePipe produces catalogues, sp_validation consumes/validates them — and puts the m-bias measurement where it belongs.

What's here

1. A self-contained m/c-bias core (src/sp_validation/image_sims.py + scripts/compute_m_bias_image_sims.py)

  • ImageSimMBias reads the five calibrated grid catalogues (reference 1z2z + the ±-shear pairs 1p2z/1m2z and 1z2p/1z2m), RA/Dec-matches them, and returns m1/m2/c1/c2 with bootstrap errors. Only internal dependency: the new match_catalogs_radec in catalog.py.
  • Paired "pool" estimator (4e38326): +g and −g catalogues are matched object-by-object to each other and differenced per object, so shape noise cancels before averaging; bootstrap resamples pairs. A guard test (test_mbias_pool_cancels_shape_noise) locks the cancellation — it fails on the naive unpaired estimator.
  • config/calibration/mask_v1.X.{4,9}_im_sim.yaml — image-sim calibration configs.
  • Synthetic recovery tests: inject known m/c, assert recovery.

2. Snakemake orchestration — the full DAG (init → pipeline → merge → extract → calibrate → m_bias) wiring both containers: ShapePipe SIF for pipeline + merge, sp_validation SIF for extract + calibrate + m-bias. Everything is parameterised through one config file (container paths, repo roots, data roots, PSF dict, tile list, sim/calibration knobs) — no hard-coded clone layout; this is the "runnable by someone other than Fabian" fix.

  • im_init now stages the raw SKiLLS inputs (input_tiles/input_exp symlinks) so im_pipeline genuinely runs from raw simulated images, not from pre-staged intermediates (58756eb).
  • The downstream (extract → calibrate → m-bias) runs on the migrated ngmix-v2 column grammar end-to-end (00ed2c4) — this flushed out a 6-defect cascade of old-grammar assumptions, all fixed on this branch.

Status: the chain runs end-to-end; the m-bias gate caught two real bugs

The full chain — raw SKiLLS images → ShapePipe → merge → extract → calibrate → m-bias — now runs on candide with a single consistent new-grammar vintage (both containers built from the current #766 / this branch). That was the point of the exercise, and the gate then did its job: the first trustworthy-conditions run returned m ≈ −1, and chasing that number down surfaced two genuine bugs in the shared calibration path, both now filed with full diagnosis:

Where this leaves the PR: the orchestration and estimator halves are done and verified; the trustworthy m-bias number itself is blocked on #226 (a design call on the shared calibration path — options laid out in the issue) and #227. Martin — the ball is in your court on those two: once the calibration route is chosen, re-running im_calibrate → im_mbias on the existing intermediates is cheap, and then we scale to the 5-grid ensemble.

— Fable, on behalf of Cail

martinkilbinger and others added 11 commits June 16, 2026 13:46
Bring the image-simulation shear-bias core over from the divergent
image_sims_val branch as a clean, self-contained unit:

- src/sp_validation/image_sims.py: ImageSimMBias reads the five
  calibrated grid catalogues (reference + g1/g2 +-shear pairs), matches
  each to the reference by RA/Dec, and returns m1/m2/c1/c2 with
  bootstrap errors. Its only sp_validation dependency is
  match_catalogs_radec.
- src/sp_validation/catalog.py: add match_catalogs_radec (nearest-
  neighbour RA/Dec match within a threshold) alongside match_stars2 --
  develop had no such helper, and image_sims_val's cat.py is not on
  develop.
- scripts/compute_m_bias_image_sims.py: CLI driver; now casts numpy
  scalars to plain floats before dumping so m_bias_results.yaml is
  human-readable rather than !!python/object binary.
- config/calibration/mask_v1.X.{4,9}_im_sim.yaml: the image-sim
  calibration/cut configs the calibrate step consumes.
- tests/test_image_sims.py: analytic synthetic-recovery test (injects a
  known m/c, asserts exact recovery) plus a match_catalogs_radec check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QcYFjEWXxyCtUGzQJ7AhLH
Add workflow/rules/image_sims.smk and a standalone entry point that drive
the image-simulation validation chain end to end across two containers,
the sp_validation-side half of UNIONS-WL/MultiBand_ImSim#1:

  ShapePipe container:  im_pipeline (raw sim tiles -> per-tile cats),
                        im_merge (create_final_cat -> final_cat_{sim})
  sp_validation container: im_extract -> comprehensive, im_calibrate ->
                        cut, im_mbias -> m_bias_results.yaml

Design:
- Native container pattern: every rule sets `container: None` and calls
  `apptainer exec` explicitly, since the two halves live in different
  images. Extract/calibrate use the sp_validation image's own
  self-consistent calibration environment; m-bias injects this branch's
  extracted core on PYTHONPATH.
- Fully parameterised under config["image_sims"] -- container paths,
  repo roots, data roots, PSF dict, tile list, sim/calibration knobs --
  so a fresh user drives it from config alone, no hard-coded clone layout
  (the "runnable by someone other than Fabian" gap).
- Shell blocks avoid f-strings, so snakemake's {input}/{wildcards}
  placeholders interpolate correctly -- fixing the {input.tiles} proxy-
  repr bug from the original Snakefile.
- params_im_sim.py derives the field name from the run-dir basename, so a
  single shared template serves every sim.
- Included in the main workflow under `if "image_sims" in config`;
  standalone via workflow/image_sims/Snakefile.

Verified on candide: DAG resolves (36 jobs, correct chaining); im_init and
im_calibrate run in-container and produce a real cut catalogue; the
extracted m-bias core produces a real m2/c2 from grid_2's calibrated
ShapePipe catalogues. Full end-to-end from Martin's on-disk intermediates
is blocked by data-vintage inconsistencies (see fiber), not the workflow.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QcYFjEWXxyCtUGzQJ7AhLH
… from scratch

im_init only staged params.py / mask / cfis, but ShapePipe's get_images_runner
resolves the raw tile+exposure images via $SP_DIR/input_tiles and
$SP_DIR/input_exp -- top-level symlinks that neither run_job nor any init
script creates. Without them im_pipeline can only run off pre-staged
intermediates, not from raw images, which defeats the "runnable from raw
SKiLLS" purpose of the workflow.

Stage input_tiles -> {input_sims_base}/{sim}/images/SP_tiles and input_exp ->
.../SP_exp in im_init, matching the ground-truth run-dir layout of Martin's
working image-sims runs. Verified: init produces resolving symlinks to the
real CFIS_simu_image-*.fits for all five grid_1 sims.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013uTmBAtdP5Wvav2M76MgCA
… grammar

The image-sim merge->extract->calibrate chain had never been run end-to-end
on the post-#761 ngmix column grammar; doing so surfaced a cascade of
old-grammar and interface defects, all fixed here (validated on 1z2z_grid_1:
2345 -> 1684 post-cut objects, <e1>,<e2> ~ 0 for the no-shear reference):

- params_im_sim.py: migrate add_cols / add_cols_pre_cal / centers to the v2
  grammar (NGMIX_ELL_* -> NGMIX_G{1,2}_*, NGMIX_Tpsf_* -> NGMIX_T_PSF_RECONV_*,
  drop NGMIX_MOM_FAIL, keep NGMIX_MCAL_TYPES_FAIL; IMAFLAGS_ISO omitted -- the
  sims run no imaging-flag masking).
- extract_info.py: guard every star-catalogue-dependent block under
  `if star_cat_path`. The sims have no star catalogue (star_cat_path=None), so
  the star match/metacal/PSF-leakage diagnostics -- all downstream of the
  comprehensive-cat write -- must be skipped rather than crash on undefined
  `ind_star`. Galaxy diagnostics preserved; mixed gal+star R histograms fall
  back to galaxy-only.
- catalog_builders.py: make CalibrateCat.read_cat FITS-aware. The sims produce
  a single per-run comprehensive catalogue (FITS, from write_shape_catalog),
  not the joined multi-patch HDF5 the data path builds; read it directly with
  dat_ext=None instead of forcing h5py.
- mask_v1.X.9_im_sim.yaml: v2 grammar (NGMIX_MOM_FAIL -> NGMIX_MCAL_TYPES_FAIL,
  NGMIX_ELL_PSFo_NOSHEAR_{0,1} -> NGMIX_G{1,2}_PSF_ORIG_NOSHEAR) and .fits input.
- image_sims.smk: comprehensive cat is .fits (write_shape_catalog always writes
  FITS); default extract/calibrate to the repo checkout and inject
  PYTHONPATH={sp_validation_repo}/src on every SPV stage, so the branch code
  (these fixes) wins over the lagging baked container.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D9atA9VPrg7VMC7oBArny9
im_init staged params.py by `cp {params.template}` where the template was a
snakemake `params:` value, not a tracked `input:`. Editing the template (e.g.
the ngmix-v2 grammar migration) therefore did NOT retrigger im_init, so run
dirs kept stale old-grammar params.py and extract died on
`no field of name NGMIX_Tpsf_NOSHEAR` -- only for sims whose run dir predated
the edit. Move template + mask_src to `input:` so a template change re-stages
into every run dir.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D9atA9VPrg7VMC7oBArny9
…tion)

The extracted ImageSimMBias._m_c_pair was UNPAIRED: it matched the +g and
-g sims each to the reference separately, meaned them independently, and
subtracted. Intrinsic shape (sigma_e ~ 0.3) never cancelled, so a single
tile gave sigma(m) ~ 1.5 -- unconstrained.

Restore the paired estimator the SKiLLS sims are built for: match the +g and
-g sims directly to each other (same galaxies, opposite input shear; verified
98.8% co-located to <0.05" with e-correlation 0.81), difference per object,

    m = <(e_+ - e_-)/(2 g_in) - 1> ,   c = <(e_+ + e_-)/2> ,

and bootstrap the paired objects (one resample applied to both sims) so the
per-object cancellation carries into the error. Intrinsic shape cancels in m;
c is a sum so it stays shape-noise limited (as it must). This also restores
Martin's direct +g<->-g matching, which the extraction (10baa52) regressed,
and goes beyond it (his bootstrap still resampled the two means independently).

New test test_mbias_pool_cancels_shape_noise builds catalogues with realistic
intrinsic scatter and asserts sigma(m) sits far below the unpaired shape-noise
floor -- the property the pooling exists to deliver. Existing exact-recovery
test still passes to machine precision.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HmxPiKJEechqueT3YU1yrx
Fresh-eyes review notes on 4e38326: document that 1z2z is the unsheared
reference (loaded for null-test diagnostics, not consumed by the +g/-g pool
estimator) and that the RA/Dec match is nearest-neighbour, not a strict
bijection -- effectively 1:1 on well-separated grid sims (~99% co-located to
<0.05" on SKiLLS grid_1) but could dilute cancellation on denser fields.
No behavioural change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HmxPiKJEechqueT3YU1yrx
@cailmdaley

Copy link
Copy Markdown
Collaborator Author

Status update (PR description rewritten to match): the chain now runs end-to-end from raw SKiLLS images on a single consistent ngmix-v2 grammar vintage, and the m-bias estimator is fixed to a paired, shape-noise-cancelling form with a guard test.

The gate then earned its keep: the first clean run returned m ≈ −1, and the diagnosis landed on two real bugs in the shared calibration path, now filed with full mechanism and evidence:

A multiplicative-only hand-diagnostic gives m₁ = +0.14 ± 0.10, m₂ = +0.27 ± 0.11 (grid_1, 1 tile), so the pipeline itself looks healthy. Once the calibration route is chosen, re-running im_calibrate → im_mbias on the existing intermediates is cheap, and we scale to the 5-grid ensemble for the real number. Ball's in your court, Martin.

— Fable, on behalf of Cail

martinkilbinger and others added 4 commits July 7, 2026 16:21
)

Fabian's #227 verdict excludes shape weights from sim m-bias calibration
(w_des is pathological on sims, N_eff ~1-5%; w_iv behaves but is also
excluded). w_col: null in the workflow config now flows through the .smk
untouched and lands here as unit weights.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvQvyJTEwYcLCwDEgpWMJ6
… the branch layout

Martin implemented the #226 fix (skip additive-bias subtraction for
constant-shear sims) on his image_sims_val branch (e38ace9) against the
old notebooks/ layout; #225 moved calibration to scripts/calibration/.
This ports the identical semantics: get_calibrated_m_c gains
additive_correction (default True, data path unchanged); the calibrate
script reads it from the mask config's metacal: section; both im_sim
mask configs set False. c/c_err still computed as diagnostics.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvQvyJTEwYcLCwDEgpWMJ6
)

The w_des-weighted global R was measured on N_eff ~ 20-100 objects and
differs from the unweighted mean by up to 17% (0.780 vs 0.669 on tile
233.293) — no weights anywhere in sim m-bias, per Fabian's #227 verdict.
calibrate_comprehensive_cat reads the key fail-fast, so both im_sim mask
configs now carry it explicitly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvQvyJTEwYcLCwDEgpWMJ6
@cailmdaley

Copy link
Copy Markdown
Collaborator Author

Three commits pushed to this branch, all exercised end-to-end on tile 233.293 (grid_1):

With all three active, plus shapepipe#803 (position-seeded fixnoise, now also on the im_sims branch with SEED_FROM_POSITION = True in the sims ngmix config), the chain runs green from raw images and gives, on one tile:

m1 = +0.274 ± 0.028,  m2 = +0.231 ± 0.022,  c1 = −0.003 ± 0.008,  c2 = −0.003 ± 0.008

The σ_m is ~5–6× smaller per tile than without the noise pairing (measured same-tile A/B; details on shapepipe#803). Percent-level σ_m now needs ~4–7 tiles.

Note the central value: the m ≈ +0.2 is now ~8–10σ from zero on a single tile. It is not estimator noise. The leading suspect is metacal's measured response: the true sample response (Δe across paired branches / 2γ) is ≈0.80 while metacal's R̄ ≈ 0.64–0.67 — a ~20% under-response. The in-memory twin passes at |m|<5e-3 with the same metacal code and an exact PSF, and its measured PSF-size-error coefficient (m ≈ +1.8·δ_size) would put a +0.2 bias at ≈ +11% PSF size mismatch — so fake_psf input fidelity is where we'll look first, then selection response.

One reconciliation heads-up for the eventual merge: image_sims_val carries Martin's parallel m-bias/diagnostics work (ddb95f0, 003058c) that this branch's scripts/ versions overlap with — worth deciding which surface is canonical before merging both.

— Claude (Fable) on behalf of Cail

…#225 branch

His side trusted, ours vetted, new grammar wins:
- image_sims.py: our paired pool estimator + paired bootstrap kept; his
  pair_match knob adopted (False = his independent-means estimator with
  independent bootstraps — the toggle switches estimator flavor, documented);
  his print_mean_ellipticities + catalog_name default adopted; grids_dir key.
- compute_m_bias_image_sims.py: ours (workflow-integrated, convergence
  tracking); his diagnostics capability preserved via
  diagnostics_image_sims.py, taken wholesale.
- catalog.py/io.py: his hdf5 write branch ported into our layout (+ the
  import os it needed and a duplicated match_catalogs_radec removed — both
  caught by the reproduction gate / pre-commit).
- calibration.py: identical additive_correction semantics both sides; our
  signature (no shape_method).
- mask configs: ours (new grammar, #226/#227 flags); his old-grammar
  versions preserved in his branch history.

Verified: calibrate+mbias rerun on tile 233.293 reproduces the pre-merge
m_bias_results.yaml BIT-EXACTLY (m1=+0.274362, m2=+0.231224, all 8 values).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvQvyJTEwYcLCwDEgpWMJ6
@github-actions

Copy link
Copy Markdown

🔴 ruff found lint / format issues

@cailmdaley — these block the merge into develop. Full list below (also surfaced as annotations in the CI run):

ruff check .

scripts/diagnostics_image_sims.py:16:1: I001 [*] Import block is un-sorted or un-formatted
scripts/diagnostics_image_sims.py:22:1: I001 [*] Import block is un-sorted or un-formatted
Found 2 errors.
[*] 2 fixable with the `--fix` option.

ruff format --check .

Would reformat: scripts/diagnostics_image_sims.py
1 file would be reformatted, 230 files already formatted

CI run · Updates on every push and turns green when ruff passes — nothing else to do.

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

Labels

None yet

Projects

None yet

2 participants