Skip to content

Commit d9aa115

Browse files
author
martinkilbinger
committed
Merge remote-tracking branch 'upstream/develop' into im_sims
2 parents 995eb6a + fc656da commit d9aa115

72 files changed

Lines changed: 2615 additions & 3035 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.felt/dependabot-pr-triage/dependabot-pr-triage.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
---
2+
id: 01KTCHWZSHQXRQHTHH9Q7Q4MT7
23
name: Triage open dependabot uv.lock PRs
34
status: closed
45
tags:
@@ -84,3 +85,7 @@ Update this fiber's `outcome:` to reflect what landed (e.g. "5 merged, 1 deferre
8485
## Skills
8586

8687
Activate `felt` (you'll touch the fiber). The `shuttle` skill is already loaded by virtue of being the dispatched worker.
88+
89+
## Follow-on: PR #742 (2026-06-11)
90+
91+
PR #742 ("chore(deps): bump the lockfile-minor-patch group, 4 updates": mpi4py 4.1.1→4.1.2, numpy 2.4.4→2.4.6, snakemake 9.20.0→9.21.0, black) arrived after the original batch. CI green (`build-test-publish` passes). Not merged — pyproject.toml modified alongside uv.lock: lower bounds tightened (`mpi4py>=4.0→4.1.2`, `numpy>=2.0→2.4.6`, `snakemake>=9.20.0→9.21.0`). Tightening `>=` floors is a policy call, not a mechanical bump. Comment posted at https://github.com/CosmoStat/shapepipe/pull/742#issuecomment-4675762821 asking maintainers to confirm intent before merging.

.felt/docker-multistage/docker-multistage.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
---
2+
id: 01KTCHWZV7263MYPNB8DT1FXPX
23
name: 'Docker multi-stage: runtime + dev targets'
34
tags:
45
- shapepipe

.felt/docker-uv-revert/docker-uv-revert.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
---
2+
id: 01KTCHWZW8DT8VRYQJ4CEPDRGB
23
name: 'Docker: revert skaha→python base, switch to uv lockfile'
3-
status: active
4+
status: closed
45
tags:
56
- shapepipe
67
- docker
78
- infra
89
created-at: 2026-04-27T11:26:45.677512058+02:00
9-
outcome: 'PR #719 (chore: switch Dockerfile to slim Python + uv lockfile) opened and CI-green on first try (3m31s); ready for Martin''s review. Drops conda double-install, makes pyproject SSOT + uv.lock the pinned manifest, switches WeightWatcher from sed-patched source build to Debian''s pre-patched 1.12+dfsg-3 package, adds binary smoke tests to deploy-image.yml.'
10+
closed-at: 2026-06-10T17:14:45.965931602+02:00
11+
outcome: 'Superseded: conda removal landed via #733 (merged); the #719-era docker-uv work is absorbed into ci-green-on-develop, which tracks remaining follow-ups.'
1012
decisions:
1113
base:
1214
label: Base image

.felt/fabian-coord-bug/fabian-coord-bug.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
---
2+
id: 01KTCHWZWEXTJ337APFSH4NF6X
23
name: Fabian's coord-propagation bug + image-sim code on github
34
tags:
45
- shapepipe

.felt/ngmix-update/ngmix-update.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
---
2+
id: 01KTCHWZWY48FVCEDX6DGE6CTY
23
name: ngmix library upgrade + Lucy wrapper sync
34
tags:
45
- shapepipe

.felt/prs-in-flight/prs-in-flight.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
---
2+
id: 01KTCHWZWYYTDSYHAP4FPE7V3V
23
name: PRs in flight after v2 merge
34
tags:
45
- shapepipe
@@ -51,7 +52,8 @@ for the one open security-fiber.)
5152

5253
| # | Author | What |
5354
|---|---|---|
54-
| #725 | aguinot | Fix centroid shift |
55+
| #741 | martinkilbinger / lbaumo | **Ngmix v2.0** — upstream ngmix 2.4.0 + Lucy's new classes. Canonical PR (CI mirror; fork PR #740 closed by Martin). OPEN, mergeable, CI green. Two-part review + next-steps triage delivered; 11 findings open (5 cut-and-dry, 5 decisions, 1 resume), 2 are merge-gates (weight-norm, `*_psfo`). See [[review-ngmix-v2-pr740]]. |
56+
| #725 | aguinot | Fix centroid shift (overlaps #741 centroid work — cross-ref/supersede?) |
5557
| #704 | martinkilbinger | Contributors |
5658
| #703 | martinkilbinger | V1.3.x |
5759
| #699 | martinkilbinger | Coverage mask |
Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
<!doctype html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="utf-8">
5+
<meta name="viewport" content="width=device-width, initial-scale=1">
6+
<title>Deep dive — ngmix v2.0 weights & size</title>
7+
<link rel="preconnect" href="https://fonts.googleapis.com">
8+
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
9+
<link href="https://fonts.googleapis.com/css2?family=EB+Garamond:ital,wght@0,400;0,500;0,600;1,400;1,500&family=IBM+Plex+Mono:wght@400;500&display=swap" rel="stylesheet">
10+
<style>
11+
:root {
12+
--parchment: #EDE2CE; --rag: #FBF6E9; --ink: #1B1611; --ink-muted: #7A6F5C;
13+
--cinnabar: #BC4538; --cinnabar-faint: rgba(188, 69, 56, 0.10);
14+
--teal: #3F8278; --cobalt: #3D5BA0; --ochre: #C49333;
15+
--rule: #C5B89E; --tooth: rgba(197, 184, 158, 0.45);
16+
--serif: 'EB Garamond', Georgia, serif; --mono: 'IBM Plex Mono', ui-monospace, monospace;
17+
}
18+
html, body { margin: 0; padding: 0; }
19+
body { background: var(--parchment); color: var(--ink); font-family: var(--serif); font-size: 19px; line-height: 1.55; -webkit-font-smoothing: antialiased; }
20+
.page { max-width: 880px; margin: 0 auto; padding: 56px 32px 72px; }
21+
.stamp { font-family: var(--mono); font-size: 11px; letter-spacing: 0.12em; text-transform: uppercase; color: var(--ink-muted); margin-bottom: 20px; display: flex; gap: 14px; align-items: center; flex-wrap: wrap; }
22+
.stamp .dot { width: 6px; height: 6px; border-radius: 50%; background: var(--cinnabar); }
23+
.stamp .sep { color: var(--rule); }
24+
h1 { font-family: var(--serif); font-style: italic; font-weight: 500; font-size: 42px; line-height: 1.1; margin: 0 0 16px; }
25+
section { margin-top: 46px; }
26+
.section-mark { font-family: var(--mono); font-size: 11px; letter-spacing: 0.15em; text-transform: uppercase; color: var(--cinnabar); margin-bottom: 8px; }
27+
h2 { font-family: var(--serif); font-style: italic; font-weight: 500; font-size: 30px; line-height: 1.2; margin: 0 0 6px; }
28+
h3 { font-family: var(--serif); font-weight: 600; font-size: 20px; margin: 26px 0 6px; }
29+
p { margin: 0 0 14px; }
30+
ul, ol { margin: 0 0 14px; padding-left: 22px; }
31+
li { margin-bottom: 7px; }
32+
code { font-family: var(--mono); font-size: 0.84em; background: rgba(196, 147, 51, 0.12); padding: 1px 5px; border-radius: 2px; }
33+
hr { border: none; border-top: 1px solid var(--rule); margin: 36px 0; }
34+
hr.short { width: 80px; margin: 30px 0; border-top-width: 2px; border-top-color: var(--cinnabar); }
35+
blockquote { margin: 14px 0; padding-left: 18px; border-left: 2px solid var(--ochre); font-style: italic; }
36+
table { border-collapse: collapse; width: 100%; font-size: 15px; margin: 0 0 16px; }
37+
th, td { text-align: left; padding: 6px 9px; border-bottom: 1px solid var(--rule); vertical-align: top; }
38+
th { font-family: var(--mono); font-size: 10.5px; letter-spacing: 0.07em; text-transform: uppercase; color: var(--ink-muted); }
39+
td.ln { font-family: var(--mono); font-size: 12.5px; white-space: nowrap; }
40+
.v-ok { color: var(--teal); font-weight: 600; }
41+
.v-warn { color: #9a6f1c; font-weight: 600; }
42+
.v-no { color: var(--cinnabar); font-weight: 600; }
43+
.lede { font-size: 21px; font-style: italic; }
44+
.metric { font-family: var(--mono); background: var(--cinnabar-faint); border-left: 3px solid var(--cinnabar); padding: 13px 16px; margin: 0 0 16px; font-size: 14px; line-height: 1.5; }
45+
.rec { background: rgba(63, 130, 120, 0.10); border-left: 3px solid var(--teal); padding: 14px 18px; margin: 14px 0; }
46+
.rec .section-mark { color: var(--teal); }
47+
details { margin: 10px 0 16px; border: 1px solid var(--rule); border-radius: 4px; padding: 8px 14px; background: var(--rag); }
48+
summary { font-family: var(--mono); font-size: 12px; letter-spacing: 0.05em; text-transform: uppercase; color: var(--ink-muted); cursor: pointer; }
49+
.tag { font-family: var(--mono); font-size: 10px; letter-spacing: 0.05em; text-transform: uppercase; padding: 2px 7px; border-radius: 10px; background: var(--cinnabar-faint); color: var(--cinnabar); }
50+
</style>
51+
</head>
52+
<body>
53+
<main class="page">
54+
55+
<div class="stamp">
56+
<span class="dot"></span><span>2026 · 06 · 05</span>
57+
<span class="sep">·</span><span>review-ngmix-v2-pr740</span>
58+
<span class="sep">·</span><span>deep dive — weights &amp; size</span>
59+
</div>
60+
61+
<h1>Two hard problems, solved.</h1>
62+
63+
<p class="lede">A fan-out of six investigators + two synthesizers chased the two science-shaped
64+
findings from the #741 review to ground. Both came back high-confidence, with verified arithmetic
65+
and — for the weight regression — an <strong>observed</strong> empirical confirmation. Each problem
66+
now has a concrete recommendation and a clean scoping plan. Full reports:
67+
<code>weights-report.md</code> and <code>size-report.md</code> in this fiber.</p>
68+
69+
<hr>
70+
71+
<!-- ============ PROBLEM A ============ -->
72+
<section>
73+
<div class="section-mark">§ Problem A · weights / inverse-variance</div>
74+
<h2>v2.0 has two real, coupled weight-map regressions.</h2>
75+
76+
<p>The live weight path is <code>prepare_ngmix_weights</code> (<code>ngmix.py:871</code>). Against
77+
the v1 baseline (<code>develop</code>), v2.0 dropped <em>two</em> things:</p>
78+
79+
<table>
80+
<tr><th>Regr.</th><th>What changed</th><th>Effect</th></tr>
81+
<tr><td class="ln">R1</td><td>Noise estimate regressed from the object-free windowed <code>get_noise</code> (still in the file at :826, now <strong>dead — zero callers</strong>) to whole-stamp <code>sigma_mad(gal)</code>, contaminated by galaxy flux.</td><td>Size/flux-dependent mis-weighting of the χ² → biased <code>g_err</code>/<code>T_err</code>/<code>s2n</code>; the signature of a multiplicative shear bias <code>m</code> (cf. old-path <code>m~-2.8e-2</code>). <em>Dominant today.</em></td></tr>
82+
<tr><td class="ln">R2</td><td>Lost the binarization <code>weight_map[weight_map != 0] = 1</code>, so line 906 multiplies the <em>raw</em> weight by <code>1/sig_noise²</code>.</td><td>For 0/1 masks: per-epoch <code>1/Fscale²</code> scale error. <strong>Latent hazard:</strong> feed a real inverse-variance map (THELI / BACKGROUND_RMS) and v2.0 double-counts the variance.</td></tr>
83+
</table>
84+
85+
<h3>Observed, not inferred</h3>
86+
<p>Fed <code>make_data</code>'s truth inverse-variance map (<code>1/noise²</code>) through the actual
87+
function in the dev container:</p>
88+
<div class="metric">
89+
noise=1e-3 &nbsp;→&nbsp; truth ivar 1e6 &nbsp;|&nbsp; recovered <b>8.81e11</b> &nbsp;|&nbsp; ratio <b>8.81e5</b> (correct = 1.0)<br>
90+
noise=1e-4 &nbsp;→&nbsp; truth ivar 1e8 &nbsp;|&nbsp; recovered <b>7.35e15</b> &nbsp;|&nbsp; ratio <b>7.35e7</b> (correct = 1.0)
91+
</div>
92+
<p>The weight is off by ≈<code>1/noise²</code> — exactly the R2 double-count. The regression is a
93+
<strong>clean red→green test target</strong> (<code>make_data</code> injects the oracle; no ngmix
94+
fit needed). Today's only weight-path test asserts <code>run(42)==run(42)</code> — determinism,
95+
never truth. Coverage gap.</p>
96+
97+
<div class="rec">
98+
<div class="section-mark">recommendation — split into two PRs</div>
99+
<p style="margin-bottom:8px"><strong>In #741 (now):</strong> minimal v1-restore — reinstate the
100+
binarization + call <code>get_noise</code> instead of <code>sigma_mad(gal)</code> — plus the
101+
red→green unit test. Two lines; fixes a regression this PR introduced; restores robustness to
102+
real ivar maps.</p>
103+
<p style="margin-bottom:0"><strong>Separate PR (next):</strong> the in-house <strong>SExtractor
104+
<code>BACKGROUND_RMS</code></strong> baseline you steered toward (THELI is external). Mostly
105+
config — the check-image &amp; vignetmaker ME loops are already list-driven, so delivering the
106+
RMS stamps is near-free; the earned work is rewriting <code>prepare_ngmix_weights</code> to use
107+
per-pixel <code>1/RMS²</code> with <code>sigma_mad</code> as fallback, + before/after validation.
108+
<strong>Effort S–M, ~4–8 h.</strong> Closes #604.</p>
109+
</div>
110+
111+
<details>
112+
<summary>Open questions for Martin (6)</summary>
113+
<ol>
114+
<li>Confirm SExtractor-BACKGROUND_RMS is the baseline, THELI a later add-on (non-blocking)?</li>
115+
<li>RMS input optional (with <code>sigma_mad</code>/<code>get_noise</code> fallback) or required? (rec: optional)</li>
116+
<li>#741 minimal fix: restore windowed <code>get_noise</code> (reintroduces the gal-guess machinery the PR may have deliberately stripped), or accept bare <code>sigma_mad</code> until #604? R1 is dominant, so it matters even short-term.</li>
117+
<li>Does the RMS map feed anything beyond weight + noise-fill (e.g. background subtraction)? (rec: weight + noise-fill only)</li>
118+
<li>Config-edit scope for #604: cfis only, or cfis + cfis_simu + canfar batch?</li>
119+
<li>Merge bar for #604: smoke before/after on the example tile, or a full sim m/c calibration?</li>
120+
</ol>
121+
</details>
122+
</section>
123+
124+
<hr class="short">
125+
126+
<!-- ============ PROBLEM B ============ -->
127+
<section>
128+
<div class="section-mark">§ Problem B · r50 / T / σ size naming</div>
129+
<h2>The v2.0 “r50” columns aren’t half-light radii.</h2>
130+
131+
<p>All five <code>r50*</code> columns are <strong>new in v2.0</strong> (v1 wrote sizes only as
132+
<code>T</code>). None is a true half-light radius:</p>
133+
134+
<div class="metric">
135+
galaxy <code>r50</code> (l.409) = <code>pars[4]</code> = <b>T = 2σ²</b> &nbsp;— an <i>area</i>, bit-for-bit a copy of <code>T</code><br>
136+
PSF <code>r50psf</code> (l.411) = <code>√(T/2)</code> = <b>σ</b> &nbsp;— a length, but off by ×1.1774<br>
137+
true half-light radius = <b>1.1774·σ = 1.1774·√(T/2)</b> &nbsp;— no column equals this
138+
</div>
139+
140+
<p>So the same name root means an <strong>area</strong> on the galaxy side and a <strong>length</strong>
141+
on the PSF side. Meanwhile the official catalogue paper (<strong>UNIONS-3500 WL I</strong>,
142+
arXiv:2605.13549) reports the <strong>half-light radius <code>r_h</code> (= r50) as the primary
143+
size</strong> for both galaxy and PSF, with the size cut written as the dimensionless ratio
144+
<code>0.707 ≤ r_h/r_h,psf ≤ 3</code>; <code>T</code> is given only as the derived DES area (Eq. 1).
145+
The paper wants exactly the quantity these columns claim but fail to provide.</p>
146+
147+
<p>sp_validation reads <strong>none</strong> of the <code>r50*</code> columns (only the
148+
dimensionless ratio <code>T/Tpsf</code>, which is robust) — so the mislabel harms nothing today,
149+
but it's a live foot-gun for any future consumer and contradicts the paper.</p>
150+
151+
<div class="rec">
152+
<div class="section-mark">recommendation — transform at the source</div>
153+
<p style="margin-bottom:8px">Make the <strong>ngmix writer</strong> emit honest columns: galaxy
154+
<code>r50 = 1.1774·√(T/2)</code> with propagated error; PSF <code>r50psf</code> = existing σ ×
155+
1.1774. Keep all <code>T*</code> columns. Then every "r50" is a length meaning the same thing on
156+
both sides, matching the paper. De-duplicate (<code>T_psfo_ngmix</code><code>Tpsf</code>,
157+
<code>r50_psfo_ngmix</code><code>r50psf</code>).</p>
158+
<p style="margin-bottom:0">Put the conversion web in <strong>cs_util</strong> once
159+
(<code>T_to_r50</code>, <code>r50_to_T</code>, <code>sigma_to_fwhm</code>, corrected
160+
<code>T_to_fwhm</code>) — the shared producer/consumer layer. Beats a pure rename (throws away
161+
the paper's headline quantity) and a read-side-only fix (leaves the foot-gun on disk).</p>
162+
</div>
163+
164+
<h3>Bonus find: a real downstream bug in sp_validation</h3>
165+
<p><code>galaxy.py:T_to_fwhm</code> is <code>T/1.17741*2.355</code> — dimensionally wrong
166+
(<code>2.355/1.17741 = 2.000</code>; it treats the area <code>T</code> as if it were σ). Its own
167+
<code>MKDEBUG</code> comment already doubts it. It builds the <code>fwhm_PSF</code> regressor for the
168+
<strong>scale-dependent PSF-leakage fit</strong> (<code>run_object.py:510</code>), so it distorts
169+
the PSF-size axis and biases <code>α(size)</code>. The correct form is
170+
<code>2.355·√(T/2)</code>. A clean cs_util surface lets this be fixed and the trap retired.</p>
171+
172+
<details>
173+
<summary>Open questions for Lucy / Martin (6)</summary>
174+
<ol>
175+
<li>Standardize on r50-as-primary (per UNIONS-3500 I) while keeping T, or keep T as working column with r50 derived?</li>
176+
<li>Drop or alias the duplicate columns — is anything outside sp_validation <code>src/</code> (notebooks, older catalogues) bound to the <code>_psfo_ngmix</code> names?</li>
177+
<li>cs_util as the home for the converter web — agreed? Naming?</li>
178+
<li>Were in-flight scale-dependent leakage results produced with the buggy <code>T_to_fwhm</code>, and how much does <code>α(size)</code> move once fixed?</li>
179+
<li>Galaxy <code>r50_err</code>: simple propagation, or from the full pars covariance?</li>
180+
<li>Catalogue-format version bump / changelog for the <code>r50*</code> semantics change?</li>
181+
</ol>
182+
</details>
183+
</section>
184+
185+
<hr class="short">
186+
187+
<section>
188+
<div class="section-mark">§ Where this leaves us</div>
189+
<h2>Two work-streams, both scoped.</h2>
190+
<ul>
191+
<li><strong>Weights → a Codex shuttle + separate PR</strong> (the #604 BACKGROUND_RMS baseline), with the minimal v1-restore landing in #741 first. The science calls above are for Martin.</li>
192+
<li><strong>Size → transform-at-source</strong> in ngmix + a cs_util converter web; the sp_validation <code>T_to_fwhm</code> fix rides along. Touches the producer/shared/consumer stack — Lucy + Martin on the convention.</li>
193+
<li><strong>Still no Martin/Lucy reply drafted</strong> — that waits for Cail. These reports are the raw material for it.</li>
194+
</ul>
195+
</section>
196+
197+
</main>
198+
</body>
199+
</html>

0 commit comments

Comments
 (0)