Complete the TransformerNUFFT migration: adjoint positional offset and Delaunay parity #326

Activity

1. Jammy2211 commented on Jul 28, 2026

   

   Jammy2211

   on Jul 28, 2026

   CollaboratorAuthor

   More actions

   Closing: neither failure reproduces, and the suspected root cause is disproven

   Re-ran both scripts on current main (PyAutoArray main, autolens_workspace_test main, autolens 2026.7.23.1) and measured the adjoint directly. Summary: there is no library-side gap here.

   1. NUFFT round-trip peak offset — passes

   Ran this issue's exact repro command verbatim:

   (d.2) round trip: peak of original = (6, 10), peak of dirty image = (6, 10), pixel distance = 0.00
   All NUFFT parity tests PASSED.
   

   And at production size (256x256, no PYAUTO_SMALL_DATASETS):

   Total visibilities: 190
   (d.2) round trip: peak of original = (141, 122), peak of dirty image = (139, 119), pixel distance = 3.61
   All NUFFT parity tests PASSED.
   

   The 5.0 -> 6.0 tolerance bump landed in autolens_workspace_test@98b2c3a ("triage 2026-05-20 cleanup: rebaseline rectangular_dspl, NUFFT tolerance, ...") on 2026-05-20 — the same day this issue was filed. The 5.00 px failure quoted above was a pre-fix run.

   The residual 3.61 px at full size is uv-coverage physics, not a convention drift: with only 190 visibilities the dirty beam has large sidelobes and the peak of a smoothed extended arc wanders. See §3 — the two adjoints are numerically identical, so no transformer convention can be responsible for it.

   2. Delaunay DatasetModel parity — passes at the size it declares

   scripts/multi/dataset_model_parity_delaunay.py:48 carries ENV: full_datasets, and its docstring states why:

   Two-frame Delaunay parity is only meaningful at full size; the 16x16 cap makes the log_likelihood floor meaningless.

   This issue's repro command forces PYAUTO_SMALL_DATASETS=1 — precisely the condition the script rules out. PyAutoHands/autohands/env_config.py:45 maps full_datasets -> unset PYAUTO_SMALL_DATASETS, so the real harness runs it uncapped. Neither script is in smoke_tests.txt.

   Run at its declared size:

   === Delaunay DatasetModel parity ===
     A0 (dataset_0, profile-baked) : log_likelihood = -1.144359e+03
     A1 (dataset_1, profile-baked) : log_likelihood = -1.144384e+03
     B0 (dataset_0, DatasetModel ) : log_likelihood = -1.144359e+03
     B1 (dataset_1, DatasetModel ) : log_likelihood = -1.144384e+03
   All four log-likelihoods agree within tolerances — Delaunay DatasetModel parity confirmed.
   

   Note also that when forced under PYAUTO_SMALL_DATASETS=1 it fails at A0 != A1 (the profile-baked baseline against itself), not the A1 != B1 this issue quotes — i.e. the capped run breaks the reference, not the DatasetModel path under test.

   3. The "suspected single root cause" is false

   Two independent disproofs:

   (a) The parity script contains no transformer. It is pure imaging — al.SimulatorImaging, al.FitImaging, a 51x51 uniform grid and a Gaussian Convolver. There is no NUFFT, no Transformer*, no visibility anywhere in it. It cannot be downstream of a NUFFT adjoint.

   (b) The adjoints are numerically identical. Measured TransformerNUFFT.image_from against TransformerDFT.image_from on random visibilities across even, odd and non-square shapes, with and without use_adjoint_scaling:

   shape	rel. max abs diff (plain)	rel. max abs diff (use_adjoint_scaling=True)	peak DFT	peak NUFFT
   (16, 16)	4.64e-12	4.64e-12	(0, 0)	(0, 0)
   (17, 17)	3.80e-13	3.80e-13	(16, 16)	(16, 16)
   (16, 20)	4.38e-12	4.38e-12	(0, 0)	(0, 0)

   No scale drift, no positional drift, no parity-dependent offset. The half-pixel shift correction in TransformerNUFFT is doing its job.

   4. The cited guard does not exist

   This issue points at "the guard in autoarray/dataset/interferometer/dataset.py:261-282". Those lines are the dirty_image = self.transformer.image_from(...) call inside apply_sparse_operator — not a guard, and not transformer-conditional. The only transformer guard in that file is the raise_error_dft_visibilities_limit check (dataset.py:115-128), which rejects TransformerDFT above 10,000 visibilities — the opposite direction. The sparse-operator path is not pinned to TransformerDFT by any code.

   Follow-up filed separately

   Two real but unrelated hygiene findings surfaced during this investigation and are being addressed on their own issue:

   1. use_adjoint_scaling is a dead parameter on both live transformers — TransformerDFT.image_from documents it "Currently unused" and TransformerNUFFT.image_from accepts-and-ignores it; only the legacy TransformerNUFFTPyNUFFT applies it. dataset.py:277 still passes use_adjoint_scaling=True, implying it is load-bearing when it is not.
   2. scripts/interferometer/nufft.py tests (a)/(b)/(c) label al.TransformerNUFFT as "pynufft", but that name now resolves to the nufftax-backed class — so max |Δ| nufftax - pynufft : 0.0000e+00 is a vacuous self-comparison and three assertions no longer test what they claim.

   Closing as not-reproducible. The TransformerNUFFT migration is complete on the adjoint side.

   🤖 Investigated via Claude Code
2. Jammy2211

   closed this as not plannedon Jul 28, 2026
3. Jammy2211

   added a commit that references this issue on Jul 28, 2026

   docs(transformer): state the real use_adjoint_scaling contract

   ...

   24863a2
4. Jammy2211 commented on Jul 28, 2026

   

   Jammy2211

   on Jul 28, 2026

   CollaboratorAuthor

   More actions

   Correction to the follow-up section above, and where both items landed

   Follow-up 1 was wrong — use_adjoint_scaling is NOT dead. Do not remove it.

   I called it a dead parameter on the strength of two of its three implementations (TransformerDFT.image_from says "Currently unused"; TransformerNUFFT.image_from says "accepted for API compatibility ... otherwise unused"). Measuring the third implementation shows the opposite — the legacy class's adjoint_scaling is compensating for pynufft's internal IFFT normalisation:

   shape	rel. err, legacy raw vs DFT	rel. err, legacy scaled vs DFT
   (16, 16)	9.99e-01	2.61e-02
   (32, 32)	1.00e+00	4.23e-03

   Raw is off by ~4*N_y*N_x (1024x, 4096x); scaled lands on the DFT to pynufft's gridding precision. So the parameter's real contract is "normalise this transformer's adjoint onto the common scale" — correctly a no-op for TransformerDFT and TransformerNUFFT (both already the plain mathematical adjoint), and load-bearing for TransformerNUFFTPyNUFFT. apply_sparse_operator is right to pass True: that is precisely what makes the sparse-operator dirty image scale-consistent across all three transformers. Deleting it would have silently broken the pynufft fallback by ~1000x.

   Fixed as a docstring correction instead, pushed to main as PyAutoArray@24863a2 — the two misleading docstrings now state the contract and carry a do-not-remove note. No behaviour change; test_autoarray/operators/test_transformer.py 15 passed.

   Follow-up 2 was real and is fixed in autolens_workspace_test@6a6156c (pushed to main). The pynufft leg of scripts/interferometer/nufft.py now names al.TransformerNUFFTPyNUFFT explicitly, so the comparisons report pynufft's genuine gridding error (2.3e-2 absolute on 5x5, 6.1e-2 relative on 256x256) rather than the vacuous 0.0000e+00. All three previously-vacuous assertions still pass — their tolerances were calibrated for real pynufft. Added lib_* coverage of the shipped al.TransformerNUFFT (forward + transform_mapping_matrix) against the local nufftax recipe so the production class has a non-vacuous assertion of its own; it matches exactly.

   No separate issue was filed — both went straight to main. This issue stays closed.

   🤖 via Claude Code