Repository navigation
IP-2b: small correctness fixes (F12, F23, D3, 585#18) #673
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
179d87f
5ca955a
c7470d8
4aa810a
f56561b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| `MultipoleField` (with `r_min_mas=0`): the taper that makes the field single-valued at the origin is now `tanh(r/length_r)^|m|` instead of `tanh(r/spacing)^|m|`, so it no longer changes with `n_r`. With the default `length_r` (five node spacings) the region where orders m >= 1 are suppressed grows from about one to about five spacings, so prior draws and every existing `MultipoleField` fit change. Also: `MultipoleDisk.suggest_n_r` emits a `UserWarning` when it steps down below the nodes it wants (return value unchanged); `ToeplitzVisData(max_offset_mas=0.0)` gives a zero bound instead of the nuisance default; already-projected `OIData` observables with NaN, infinite or non-positive errors now raise `ValueError`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -573,13 +573,16 @@ def polar_values(self): | |
| ) | ||
| if self.r_min_mas == 0: | ||
| # A smooth single-valued field has a_m(r) ~ r^|m| at the origin | ||
| # (Born & Wolf 1999, sec. 9.2): scale order m by tanh(r/spacing)^|m|, | ||
| # which is 1 - O(e^(-2r/spacing)), so the prior stays stationary | ||
| # away from the centre and only m = 0 survives at r = 0. | ||
| # (Born & Wolf 1999, sec. 9.2). The power law holds out to the | ||
| # scale on which the field varies, its radial correlation length | ||
| # ell (a physical length: the node spacing is only the grid's, so | ||
| # the taper would change with n_r). Scale order m by | ||
| # tanh(r/ell)^|m|, which is 1 - O(e^(-2r/ell)), so the prior stays | ||
| # stationary away from the centre and only m = 0 survives at r = 0. | ||
| n_theta = field.shape[1] | ||
| j = np.arange(n_theta) | ||
| m = np.minimum(j, n_theta - j) | ||
| taper = np.tanh(nodes / self.spacing)[:, None] ** m[None, :] | ||
| taper = np.tanh(nodes / self.length_r)[:, None] ** m[None, :] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change leaves The factor is also more than "1 − O(e^(−2r/ℓ))" for the high orders that
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| field = np.real( | ||
| np.fft.ifft(taper * np.fft.fft(field, axis=1), axis=1) | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import numpy as onp | ||
| import pytest | ||
|
|
||
| from virgil.oidata import OIData | ||
|
|
||
|
|
||
| def _record(vis, d_vis): | ||
| n, m = 8, 5 | ||
| rng = onp.random.default_rng(0) | ||
| return { | ||
| "u": rng.uniform(1.0, 5.0, n), | ||
| "v": rng.uniform(1.0, 5.0, n), | ||
| "wavel": onp.full(n, 2.2e-6), | ||
| "vis": vis, | ||
| "d_vis": d_vis, | ||
| "vis_mat": rng.normal(size=(m, n)), | ||
| } | ||
|
|
||
|
|
||
| def test_projected_data_build_when_finite(): | ||
| OIData(_record(onp.full(5, 0.9), onp.full(5, 0.01))) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "vis, d_vis", | ||
| [ | ||
| (onp.array([0.9, onp.nan, 0.9, 0.9, 0.9]), onp.full(5, 0.01)), | ||
| (onp.full(5, 0.9), onp.array([0.01, onp.inf, 0.01, 0.01, 0.01])), | ||
| (onp.full(5, 0.9), onp.array([0.01, -0.01, 0.01, 0.01, 0.01])), | ||
| (onp.full(5, 0.9), onp.array([0.01, 0.0, 0.01, 0.01, 0.01])), | ||
| ], | ||
| ) | ||
| def test_projected_data_must_be_finite_with_positive_errors(vis, d_vis): | ||
| with pytest.raises(ValueError, match="already projected"): | ||
| OIData(_record(vis, d_vis)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This page is generated from$\Delta r$ the node spacing.
notebooks/multipole_disk_rings.ipynb(scripts/sync_tutorial_docs.pyMAPPINGS, 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)withtests/lint/test_tutorial_docs_sync.pywill fail.The bigger problem: the figure under this paragraph is now wrong. The notebook uses
length_r=6.0on 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 theawaiting-ozstarlabel if it is heavy), and then run the sync script.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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