[benjaminpope]

IP-2b of the 0.4.2 open-issues plan. Closes findings #604 F12, #604 F23, #604 D3 and #585#18.

Fixes (each test fails on main 6b80f75, passes here)

• F12 (radio_toeplitz.py): max_offset_mas=0.0 is honoured (is None test, not falsiness). Test: test_a_zero_max_offset_is_honoured_....
• F23 (models.py): MultipoleDisk.suggest_n_r warns when the rule steps down. Test: test_suggest_n_r_warns_when_it_steps_down.
• D3 (fields.py): the MultipoleField origin taper is tanh(r/length_r)^|m|, not tanh(r/spacing)^|m|. Born & Wolf's a_m ~ r^|m| holds out to the scale on which the field varies, its radial correlation length. The node spacing is the grid's, so it changed with n_r. Derivation is in the code comment; docs sentence updated. Test: same physical field on n_r = 9 and 17 gives the same taper-variance ratio, (tanh(4/ell)/tanh(8/ell))^(2m). Two existing tests now set length_r explicitly to the old scale.
• 585#18 (oidata.py): already-projected observables (size != n_samples) skipped all validation. They must now be finite with finite non-negative errors, else ValueError (zero projected errors are still rejected where data are whitened). Tests: tests/test_oidata_projected.py.

Changes to default behaviour

1. MultipoleField with r_min_mas == 0: the taper scale changes from the node spacing to length_r (default 5 spacings), so the m >= 1 harmonics are suppressed over a wider radius by default. Prior draws and any fit with a MultipoleField change.
2. suggest_n_r emits a UserWarning where it used to step down silently (return value unchanged).
3. OIData built from projected data containing NaN/Inf/negative errors now raises (previously accepted).
4. max_offset_mas=0.0 now gives a zero shift bound instead of the nuisance default.

Verified, not bugs

• 620#9 (star.py pinv, zero diameter prior): not a bug. J^T J + diag(0, 1/s^2) is singular only when the diameter column of J is exactly zero; the diameter then decouples (P_ds = 0), so the null space is exactly the diameter axis and pinv gives the exact shape marginal. Script compared pinv with an inverse using a proper vanishing diameter prior on four cases (zero column, both zero, collinear ridge, generic): identical to 1e-16; min diagonal 0.0, never negative.
• 585#16, extra-observables half: not a bug. Extra blocks (OI_FLUX, T3AMP, VISAMP, VISPHI) take no fitted noise scale (inflated_errors leaves them unchanged), so there is no scale parameter to carry Fisher information; their mean rows are in _whiten_with_log_norm(frozen=True). The quadratic sqrt2 log sigma half is likelihood.py, outside this PR's files (IP-2a area).
• 585#19 (loose test tolerances): not changed here (test files belong to IP-3). test_closure.py:261/267: the tolerances bracket Monte-Carlo error of 4000 draws (relative sd of a variance is sqrt(2/4000) = 2.2%, so rtol 0.1 is 4.5 sigma; sd of a covariance entry is about 1/sqrt(4000) = 0.016 so atol 0.1 is 6 sigma). They are statistically justified but loose; tightening to about 4 sigma is IP-3's call. test_composition.py:449 has a stated float32 reason. test_aliases.py:1415 not examined further.
• 620#11: not vacuous as claimed. chi2 terms are non-negative, so sum < 1e-3 bounds every term and no cancellation can hide an error; sum > 1e3 for the wrong node can only pass if some term is large. The ordering assert in test_aliases.py:1128 is weak but also cancellation-free. Test files, left to IP-3.

Notes

• Needs a notebook re-run: the figures in notebooks/multipole_disk_rings.ipynb / docs/multipole_disk_rings.md (MultipoleField prior draws, a_m tracks) were rendered with the old taper (scale 1.29 mas; now length_r = 6 mas). The prose is updated; re-execute the notebook (OzSTAR if heavy) and run scripts/sync_tutorial_docs.py before release. test_latents_are_whitened_... now uses an annulus (the taper lowers variance near r = 0 by design; tolerance unchanged).
• Main may be red from main is red #662 (test_kkt_report); Fix main red: LinearImage brightness is NaN at all-zero pixels, stopping L-BFGS-B (#662) #666 fixes it. Not touched here.
• Tests run locally were the targeted ones only (toeplitz, multipole disk/field, projected OIData); CI runs the full suite.

Does any change pass by special-casing, tuning or loosening? No. No tolerance, seed or target was touched; the length_r scale is derived from the field's own correlation length, and the new tests were shown to fail on main.

🤖 Generated with Claude Code

[github-actions]

Size of this PR (Python lines)

	added	removed	net
src/	+41	-24	17
tests/	+90	-7	83

metric	main	this PR
private defs referenced once (raw text count, not the gate)	237	237
private defs	1046	1046
docstrings of 25-40 lines	157	157
docstrings over 60 lines	0	0
mean docstring lines	8.71	8.71

[claude]

Claude encountered an error after 3m 26s —— View job

---

Review of PR #673

I'm asking for changes. The max_offset_mas=0 (F12) fix is correct. The other three fixes need work: one has a correctness gap, and two leave docs, changelog or a test short. Details are in the six inline comments.

What to fix

1. Wrong result (oidata.py:2043): the new check uses errors >= 0, so already-projected data with a zero error still build. Projected data skip _diagonalized, so nothing catches the zero later, and whitening divides by it: inf/NaN χ² and gradients. The new docstring says the opposite. Use _valid_samples (> 0) and add a zero-error test case.
2. Stale docs
   • docs/multipole_disk_rings.md is generated from notebooks/multipole_disk_rings.ipynb, but only the .md was edited. The docs-sync lint test will fail.
   • That notebook's prior-draw and a_m(r) figures were rendered with the old taper. Its taper scale grows about 4.6×, so it needs re-executing.
   • docs/api/models/notes.md:393-401 still says the taper uses the node spacing, and that the prior is stationary beyond five spacings. With the new default it is about 25 spacings.
3. Undocumented default changes: changelog.d/README.md has a default fragment type for this. The wider default MultipoleField taper, which changes prior draws and every r_min_mas=0 MultipoleField fit, and the new suggest_n_r warning belong in a .default.md fragment, not only the .fixed.md bullet.
4. Weak test (test_multipole_field.py:843): at r = 4, main's value (≈0.996) is only ~0.03 above the accepted band [0.645, 0.967]. That gap is within about 1 sd of the Monte-Carlo noise, so another seed could let main pass. Comparing at the r = 1 node separates the fix from main by a factor of 20–80.
5. Minor (models.py:4784): the warning can claim 16 nodes build without having tried 16. Use external_stacklevel() instead of stacklevel=2, and mention the warning in the docstring.

Checked, no problems found

• F12: a zero bound works with _shift's error_if.
• Taper physics: ℓ_r is a reasonable grid-independent scale, and the existing tests were adjusted without loosening any tolerance.
• Not run: I couldn't run any code in this environment, so I didn't check numerically that the gradient with respect to length_r is finite at the r = 0 node with m = 0. JAX's integer-exponent pow derivative should handle that case.
• AGENTS.md: no public doc links to design/, and the anti-reward-hacking answer in the description holds.

[claude]

>= 0 lets a zero error through, and nothing later rejects it for already-projected data. _transform_observed_channels skips data whose size differs from n_vis/n_phi (oidata.py:984, :1005), so _diagonalized and its _ZERO_PROJECTED_ERROR check never run on this path. The new docstring says "a zero projected error is rejected where the data are whitened", but I couldn't find any such check for this case.

Failure: OIData({..., "vis": [0.9]*5, "d_vis": [0.01, 0.0, 0.01, 0.01, 0.01], "vis_mat": A}) builds, and then whitened_residuals divides by zero, giving inf/NaN χ² and a NaN gradient. Projected values are already the whitened basis, so a zero error is no more usable here than on the non-operator path. That path uses _valid_samples (> 0).

Suggested change

	and onp.all(onp.asarray(errors) >= 0)
	if not onp.all(_valid_samples(values, errors)):

(and change the message and docstring to say "positive"). tests/test_oidata_projected.py has no zero-error case, so add onp.array([0.01, 0.0, 0.01, 0.01, 0.01]) to the parametrize list.

[benjaminpope]

Switched to _valid_samples (positive finite errors), message and docstring say positive, zero-error case added to the parametrization. 4aa810a

🤖 Addressed by Claude Code

[claude]

This page is generated from notebooks/multipole_disk_rings.ipynb (scripts/sync_tutorial_docs.py MAPPINGS, line 64), so AGENTS.md says not to edit it directly. The notebook's markdown cell (around line 603 of the .ipynb) still says \tanh^m(r/\Delta r) with $\Delta r$ the node spacing. tests/lint/test_tutorial_docs_sync.py will fail.

The bigger problem: the figure under this paragraph is now wrong. The notebook uses length_r=6.0 on 32 nodes over 40 mas (Δr ≈ 1.29 mas), so the taper scale goes from 1.29 to 6 mas, about 4.6× wider. The "pinch to zero at r = 0" in the a_m tracks and the prior draws were rendered with the old taper. Edit the notebook cell, re-execute it (an OzSTAR run plus the awaiting-ozstar label if it is heavy), and then run the sync script.

[benjaminpope]

Edited the notebook cell and regenerated the page (sync test passes). The figures were rendered with the old taper and need a re-execution of the notebook (heavy; left for an OzSTAR run, flagged in the PR body). 4aa810a

🤖 Addressed by Claude Code

[claude]

This change leaves docs/api/models/notes.md:393-401 (MultipoleField, "Regularity at the centre") stale. That section still says $\tanh(r/\Delta r)^{|m|}$ with $\Delta r$ the node spacing, and it says "the variance of order $m$ is within $4|m|e^{-10}$ of $\sigma^2$ beyond five spacings". Since the default length_r is 5 spacings, the default prior now becomes that close to stationary only beyond about 25 spacings, which is past the edge of a 16- or 32-node grid.

The factor is also more than "1 − O(e^(−2r/ℓ))" for the high orders that n_theta allows: the variance is tanh(r/ℓ)^(2|m|), so for n_theta = 64 (as in the tutorial), m = 32 at r = 2ℓ keeps only 0.964^64 ≈ 10% of its variance. The physics argument for ℓ_r is fine, since it is grid-independent. But the docs should state that the default region of non-stationarity grows from ~1 to ~5 spacings, and that a fitted length_r now also sets how much m ≥ 1 structure the inner disk can carry.

[benjaminpope]

Updated docs/api/models/notes.md: taper is tanh(r/ℓ_r)^|m|, the default non-stationary region grows from ~1 to ~5 spacings, high orders recover slowly (n_θ=64, m=32 keeps ~10% at r=2ℓ_r), and a fitted ℓ_r sets inner m≥1 structure. 4aa810a

🤖 Addressed by Claude Code

[claude]

changelog.d/README.md has a default type for "changes to default behaviour" (see +outlier-mixture.default.md). The PR body lists four default changes, but this .fixed.md bullet presents them only as fixes. The one users will notice is the MultipoleField taper. "No longer changes with n_r" does not tell a reader that the default suppression radius grows from one node spacing to length_r (five spacings), so prior draws and every existing r_min_mas=0 MultipoleField fit change. The fragment also doesn't mention the new suggest_n_r UserWarning. Split out a +ip2b-small-fixes.default.md that states these consequences.

[benjaminpope]

Split into +ip2b-small-fixes.default.md stating the taper consequence (suppression radius one spacing → length_r, prior draws and fits change) and the suggest_n_r warning. 4aa810a

🤖 Addressed by Claude Code

[claude]

At r = 4 the test barely tells the fix apart from main. Expected is (tanh 2 / tanh 4)^6 ≈ 0.806, so rel=0.2 accepts [0.645, 0.967]. Main's spacing taper gives (tanh 4/tanh 8)^6 ≈ 0.996 (n_r = 9) and ≈ 1.0 (n_r = 17), only ~0.03 above the upper bound. The Monte-Carlo sd of a ratio of two 800-draw |FFT|² means is a few percent (~0.03–0.04 here), so main's result is within about 1 sd of passing. The test fails on main with seed 0, but another seed could let it pass.

Use the r = 1 node (index (n_r - 1) // 8: 1 and 2). There the expected ratio is (tanh 0.5 / tanh 4)^6 ≈ 0.0098, while main gives tanh(1)^6 ≈ 0.19 (n_r = 9) and tanh(2)^6 ≈ 0.80 (n_r = 17). That is a factor of 20–80 off, and the two grids also disagree with each other on main, which is the n_r-dependence this test means to catch.

[benjaminpope]

Moved to the r = 1 node: expected 0.0098 against main's 0.19 (n_r=9) and 0.80 (n_r=17). 4aa810a

🤖 Addressed by Claude Code

[claude]

Minor. (1) If no rule above 16 builds, the loop exits at n = 16 without trying 16, so "can build at most 16" is claimed without being checked. (2) stacklevel=2 points at the caller, which is pipeline/disk.py:568 when the pipeline sizes nodes. The repo already has external_stacklevel() for this (used at oidata.py:331). (3) The docstring doesn't mention the new warning; one line would do.

[benjaminpope]

Reworded the warning so it no longer claims a checked maximum, uses external_stacklevel(), and the docstring mentions it. 4aa810a

🤖 Addressed by Claude Code