Skip to content

fix(LU): preserve transpose state when conjugating - #263

Merged
jpbrodrick89 merged 2 commits into
patrick-kidger:devfrom
AHMETHAKANBEZIR1:fix/lu-conjugate-state
Oct 27, 2026
Merged

jpbrodrick89 merged 2 commits into
patrick-kidger:devfrom
AHMETHAKANBEZIR1:fix/lu-conjugate-state

Conversation

@AHMETHAKANBEZIR1

@AHMETHAKANBEZIR1 AHMETHAKANBEZIR1 commented Oct 16, 2026 •

Copy link
Copy Markdown

LU.conj conjugates the numerical factors but also flips the state's transpose flag. It consequently solves conj(A).T instead of conj(A), contrary to the solver-state contract.

Preserve the existing transpose flag when conjugating the LU factors, as the QR solver already does. This handles conjugation after transposition and repeated conjugation without changing the stored pivots or packed structures. Targets dev; no documentation/API/dependency changes.

The regression now follows the maintainer's requested scope: the separate Normal(LU) gradient test has been removed. The state-transform test uses all solver/tag variants from the existing test registry; supported singular systems use the existing singular-matrix generator and a pseudoinverse reference. Other cases use jnp.linalg.solve. All solvers cover real/complex double precision, with additional LU single-precision coverage, eager/JIT, and all six transpose/conjugation orders.

Validation on dev d6c8903, Python 3.12.14 / Windows CPU:

  • 864 state-transform cases; final LU subset against the clean base: 24 failures and 24 passing controls.
  • Entire solve module: 897 passed on JAX 0.11.2 and on minimum supported JAX 0.10.1.
  • All configured hooks, including full Pyright and CI test-group coverage, passed, both for all files and at commit time.
  • The original production-fix revision completed the configured full suite: 6585 passed / 2 existing skips across 18 isolated file runs. This follow-up changes only tests/test_solve.py, whose entire updated module passes as above. A duplicate full rerun completed its first five modules without failures before being deliberately stopped; it is not claimed as a completed full run for this test-only revision.

GPU/distributed execution was not tested. AI disclosure: prepared and tested autonomously with OpenAI Codex; no independent human pre-submission review was performed.

@jpbrodrick89

Copy link
Copy Markdown
Collaborator

Thanks for spotting! The reason thos was never caught was because conj is only exercised on the jvp path for rank deficient solvers. However, Normal(LU) is a rather contrived example as users would usually opt for a Hermitian inner solver like Cholesky when using Normal. I think it would be better to remove the second test and juat parameterose the first over all solvers. Be careful about using the right reference for pseudo inverse solvers.

@jpbrodrick89
jpbrodrick89 merged commit b5c0db5 into patrick-kidger:dev Oct 27, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants