fix: dispatch PyLinearSolver::solve to the Python solve method - #95
Merged
Conversation
The solve override in the PyLinearSolver trampoline used PYBIND11_OVERRIDE_PURE(bool, T, factorize, ia, ja, a, b, x), naming 'factorize' instead of 'solve'. When Problem::hm_inv_v called solve on a LinearSolver subclassed in Python, the trampoline invoked the Python 'factorize' method (with 5 arguments) and never the user's 'solve', so Python-defined linear solvers did not work. Dispatch to 'solve'. Add a regression test that subclasses LinearSolver in Python and asserts, via Problem.hm_inv_v, that solve is invoked with the correct right-hand side and its result flows back.
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.
Problem
The
solveoverride in thePyLinearSolvertrampoline dispatched to the wrong Python method:When
Problem::hm_inv_vcallssolve(ia, ja, a, b, x)on aLinearSolversubclassed in Python, the trampoline invoked the Pythonfactorizemethod (with 5 arguments) instead of the user'ssolve. A Python-defined linear solver therefore never had itssolveexecuted.Fix
Name
solvein the macro, so the override dispatches to the Pythonsolve.Test
New
tests/test_linear_solver_trampoline.pysubclassesLinearSolverin Python (recording how it is called and applying a trivialx = b + 1), assigns it asproblem.linear_solver, and callsproblem.hm_inv_v. It asserts thatsolveis dispatched exactly once with the correct right-hand side, thatfactorizeis only ever called with its 3-argument signature, and that thexwritten bysolveflows back to the caller.solveis never called (factorizeis invoked in its place); test fails.