Fix wrong V-pol (IV) imaging gradient: chisqgrad_vvis drops the rho gradient

[rohandahale]

Fixes a real correctness bug in IV (Stokes I + circular pol) imaging: the analytic gradient chisqgrad_vvis dropped the physical ρ-gradient, making the V-angle gradient ~99% wrong. NumPy IV reconstructions have been converging against a bad gradient.

Root cause

pol_solve masks physical gradient rows inside chisqgrad_vvis, but the vcv transform is non-diagonal — its Jacobian couples the V solver variable (vprime) to both physical ρ and ψ:

vcv_grad.out[2] = drho_dvprime * grad_rho  +  dpsi_dvprime * grad_psi

For pol='IV' (pol_solve=[1,0,0,1]), chisqgrad_vvis computed only rows 0 and 3 — leaving grad_rho (row 1) = 0 — so the dominant drho_dvprime · grad_rho term was dropped.

Why the other pol modes escaped:

• IP (mcv): the un-computed row is ψ, but its coupling coefficient dpsi_dmprime = 0 at vfrac=0 → harmless.
• IPV (polcv): the transform is diagonal and pol_solve[1]=1 anyway → ρ-gradient is computed.

So the bug is IV-specific: only vcv has a nonzero coupling to the row pol_solve skipped.

Fix

Compute the physical ρ-gradient whenever V is solved (pol_solve[1] or pol_solve[3]), in both chisqgrad_vvis and its nfft twin chisqgrad_vvis_nfft. Two-line condition change + comments.

Validation

• IV objgrad now matches central finite differences to median 2.8e-10, max 1.6e-9 (was ~99% wrong).
• New FD-based regression test test_iv_gradient_matches_finite_difference.
• The 26 existing pol chisqgrad parity tests (direct + nfft) and the IP recon test are unchanged (the fix is consistent across direct + nfft, so parity holds). Ruff clean.

How it was found

Surfaced by the JAX autodiff objective port (#295): jax autodiff and finite differences agreed, while the hand-written analytic disagreed — the same discovery mechanism as the stv_pol_grad fix (#240).

Notes

• This affects released numpy IV imaging, so like Fix factor-of-2 bug in stv_pol_grad gradient #240 it targets dev for a fast main release. Rebased onto dev locally so the diff is just the fix (not the jax development on dev-backend); once merged it can propagate dev→dev-backend→dev-backend-mixpol and be cherry-picked to main.
• Audit done (chisqgrad_p/chisqgrad_m, follow-up to this fix): IP (mcv), IQUV (polcv), and IV (vcv, post-fix) all match finite differences to ~1e-9. The same-class fragility in chisqgrad_p/m is latent only — mcv is always used at vfrac=0, where its psi coupling coefficient is exactly 0 — so they are correct in every real mode and no code change is needed. This PR is scoped to the reproduced IV fix only.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.75%. Comparing base (cec8b7d) to head (d9c0527).

Files with missing lines	Patch %	Lines
ehtim/imaging/pol_imager_utils.py	0.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #296      +/-   ##
==========================================
- Coverage   43.75%   43.75%   -0.01%     
==========================================
  Files          52       52              
  Lines       26289    26289              
  Branches     4473     4473              
==========================================
- Hits        11504    11503       -1     
  Misses      13523    13523              
- Partials     1262     1263       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[achael]

Good catch -- another bug from my major 2024 reorg or the polarization imaging code that didn't surface because it's on a non-standard imaging path. 2 questions.

1. I think that the pol_solve entries are meant to directly correspond to the Poincare sphere slots (I,rho,phi,psi). So the logic in IV imaging I think should not be to change the if statements in chisqgrad_vvis, but to change which pol_solve entries are assigned to IV imaging to be (1,1,0,1). Does that make sense?

2. we now have 4 versions of the code that all should in principle get this fix -- the main branch (stable, 1.3.1), the dev branch (1.4, pre-jax), the dev-backend branch (jax) and the dev-backend-mixpol branch. I agree all of these should get the fix, but I don't think any of them should have a major change of scope yet. So it would be useful to think about what the best strategy is to propagate this fix -- should we just have separate PRs for each or is there a better way?

[rohandahale]

@achael Thanks!

1. pol_solve=(1,1,0,1) for IV: I tried it, and it isn't clean as-is, because which_solve is doing two things: pack_imarr builds the solver vector from its nonzero slots, and that same array is what gets passed to chisqgrad_vvis as pol_solve. So changing index 1 on to get the rho-gradient also makes m a solved variable:

• IV which_solve = [1,0,0,1] --> 2 solver images (I + V)
• (1,1,0,1) --> 3 solver images (I + m + V)

Since vcv holds m = P/I fixed, that extra m is a unnecessary in IV imaging.

I agree the Poincaré method is the right way. The issue is that one array currently means both "which slots are solver variables" and "which physical gradients to compute". IV wants those to differ (solve I + v′, but compute the physical rho-gradient that vcv_grad uses). The if pol_solve[1] or pol_solve[3] change gives that with no extra variable. Doing it your way cleanly would mean splitting which_solve into a solver selection array vs an active physical slots array. I am happy to file that as a follow-up (it is like with the imager-config NamedTuple work).

2. It's a two line fix (chisqgrad_vvis + the nfft variant), so rather than four hand-written PRs I would do what we did for Fix factor-of-2 bug in stv_pol_grad gradient #240: a minimal fix on dev, let it merge dev --> dev-backend --> dev-backend-mixpol, and if you want, cherry-pick to main (1.3.1). I can retarget this PR from dev-backend to dev.

[achael]

great, makes sense -- i think your proposed merge sequence also makes. sense.

The base branch was changed.

[achael]

@rohandahale it looks like rebasing it from github to dev introduces a bunch of other changes from the jax development, so i'll wait for you to do that locally. Than we propagate the change from dev->dev-backend and from dev->dev-mixedpol, and we cherry pick for main.

[rohandahale]

@achael Done, ready to merge. Then we can do dev-->dev-backend & dev->dev-backend-mixpol and only these two lines into main.