pol gradient fixes + tests to main (v1.3.2)#307
Merged
Conversation
Pre-existing bugs in pol-gradient slots that only run when Stokes I is solved together with polarization (normally I is held fixed, so these never executed). Found while building gradient tests: - chisqgrad_m: restore dropped mimage*exp(-2i*chi) factor in dchi2/dI (was `np.real( * ...)` -> TypeError) - smgrad: outgrad -> gradout (NameError) - svfluxgrad: gradoutout -> gradout (NameError) - make_chisq_dict: pass the Stokes-I row to chisq for standard data terms in pol mode, matching the (already-correct) gradient path
Start of tests/test_gradients_main_tmp.py — a temporary suite validating main's gradients before dev replaces main (mirrors PR #306's gradient tests against main's API). This commit: fixtures + tolerances + nfft gate, and intensity chi^2 gradient-vs-FD checks for all DATATERMS (direct+nfft). The two diagonalized-closure cases fail until the #233 NumPy>=1.24 chisqdata fix is ported (next commit). Also drop the `*test*` gitignore rule (it hid the top-level tests/ dir) and replace it with an explicit .pytest_cache/ ignore.
main's diag-closure chisqdata/chisqgrad crash on NumPy >= 1.24:
- np.array() on ragged per-timestamp matrices -> add dtype=object in the
six chisqdata_{cphase,logcamp}_diag{,_fft,_nfft} builders
- np.float (removed in 1.24) -> float in the six diag-gradient normalizers
Mirrors dev's #233 (minus dev's unrelated nfft block_diag restructure).
Unblocks the cphase_diag / logcamp_diag gradient tests (now passing).
Adds to test_gradients_main_tmp.py: - intensity chi^2 nfft-vs-direct agreement (value + relative-L2 gradient) - intensity regularizer gradient vs FD - pol chi^2 gradient vs central FD (all 4 slots; vvis EVPA slot == 0), direct + nfft, plus pol chi^2 nfft-vs-direct agreement - pol regularizer gradient vs central FD (all 8 REGULARIZERS_POL) 51 passing. logamp uses debias=False (noise-free obs); pol fixtures use a square grid (add_random_pol scattering rect bug on main).
Completes test_gradients_main_tmp.py: - boundary FD for ptv/vtv/tv (pins the #306 first-row/col mask fix) - transform_gradients chain-rule vs FD for mcv/vcv/polcv, incl. the off-diagonal cross-terms and log-I-slot preservation - end-to-end objective FD (objgrad vs FD of objfunc) sampling the pol DOF block for IP/mcv, IV/vcv, IQUV/polcv x {direct, nfft} Full suite: 71 passing.
- gate nfft cases on pynfft (what main's nfft ttype actually uses), not the unrelated `nfft` PyPI package - build pol fixtures manually (smooth + jittered Q/U/V) instead of add_random_pol, whose scattering phase screen calls scipy.integrate.quad in a way that breaks on newer scipy (and on rectangular grids); pol fixtures return to 32x48 rect 71 pass (system, with pynfft); 44 pass + 12 nfft-skip without pynfft.
Add a test job to .github/workflows/ci.yml running only tests/test_gradients_main_tmp.py on Python 3.8 and 3.11. pynfft is not installed in CI, so the nfft cases skip and the direct-FT cases run.
rohandahale
approved these changes
Jun 16, 2026
rohandahale
left a comment
rohandahale
left a comment
Collaborator
There was a problem hiding this comment.
I reviewed and ran the gradient tests locally and all of them pass. Looks good to merge!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Patches a latent bug in the polarimetric imager gradients on
main,mirroring the dev fix in #306, and fixes several further latent
pol-gradient bugs uncovered while adding test coverage. Ships an interim
gradient-validation suite and wires it into CI.
Core fix (mirrors #306)
physical_grad_slotsseparates the physical-gradient mask from thesolve mask, centralizing the mcv/vcv cross-coupling (one solved pol DOF
drives BOTH rho and psi). Fixes IP imaging when V!=0 (and the IV analog).
mcv_grad/vcv_grad: add the off-diagonal Jacobian terms (dpsi/dm',drho/dv'); return 3-wide and assign to outarr[1:4] so the Stokes-I log
Jacobian survives. Same preservation for the polcv path.
stv_pol_grad(ptv): first-row/col back-neighbor masking (corner was~4x off vs finite differences).
Further latent bugs fixed (found via the new tests; all pre-existing, all in never-exercised Stokes-I / V-flux slots)
These paths are already correct on dev -- its gradient test coverage caught
them; this PR backports the equivalent fixes to main.
chisqgrad_m: restore droppedmimage*exp(-2i*chi)factor (was aTypeError from
np.real( * ...)).smgrad/svfluxgrad:outgrad/gradoutout->gradout(NameError).make_chisq_dict: pass the Stokes-I row to chisq for standard dataterms in pol mode (the gradient path was already correct).
#233 port
dtype=objecton ragged diag arrays,np.float->float) so the diagonalized closures build.Tests + CI
tests/test_gradients_main_tmp.py: interim gradient suite (71 tests) --nfft-vs-direct, analytic-vs-FD for all intensity/pol chi^2 terms and
regularizers, and the mcv/vcv/polcv transforms vs FD (isolated +
end-to-end through the Imager objective). Temporary; delete when dev
lands on main.
testjob on py3.8 + py3.11 running the suite (pynfft absent in CI-> nfft cases skip, direct-FT cases run).
*test*gitignore rule (it hid top-level tests/).Version
How to test
python3 -m pytest tests/test_gradients_main_tmp.pySystem python (has pynfft): 71 pass. Without pynfft: 44 pass + 12 nfft-skip.
Design decisions
from dev.
scipy-version-fragile scattering path (and the rect-image scattering bug).