Repository navigation
fix(LU): preserve transpose state when conjugating - #263
Merged
jpbrodrick89 merged 2 commits intoOct 27, 2026
Merged
Conversation
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. |
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
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.
LU.conjconjugates the numerical factors but also flips the state's transpose flag. It consequently solvesconj(A).Tinstead ofconj(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 usejnp.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
devd6c8903, Python 3.12.14 / Windows CPU: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.