Make LinearSolver a subclass of LinearVariationalSolver

[pbrubeck]

Description

Provides enhacements for LinearVariationalSolver to support more generic MatrixBase operators, and makes LinearSolver a subclass of LinearVariationalSolver.

Fixes #4016

[github-actions]

	Tests	Passed ✅	Skipped ⏭️	Failed ❌
Firedrake real	7686 ran	7051 passed	635 skipped	0 failed

[github-actions]

	Tests	Passed ✅	Skipped ⏭️	Failed ❌
Firedrake complex	8112 ran	6556 passed	1555 skipped	1 failed

[connorjward]

Rather than doing this would it not be possible to deprecate LinearSolver in favour of LinearVariationalSolver? I don't think wrapping one in the other makes sense in terms of code design.

[pbrubeck]

There's still good use for this class, especially because it enables the user to provide multiple right-hand side and solution pairs. Another good reason is that this gives the user another way of preventing the Jacobian of being re-assembled.

[pbrubeck]

This PR is also paving the way for composing Fieldsplit and other solvers to solve systems with pre-assembled matrices and also more general BaseForms.

[connorjward]

This PR is also paving the way for composing Fieldsplit and other solvers to solve systems with pre-assembled matrices and also more general BaseForms.

But why is LinearVariationalSolver not suitable for those cases? We can now represent assembled matrices in UFL so can't the code paths be unified? I think @JHopeCollins has had some success with this sort of thing using external operators (when we were discussing taping for LinearSolver).

gives the user another way of preventing the Jacobian of being re-assembled.

Surely multiple methods to achieve the same result suggests that things should be combined?

it enables the user to provide multiple right-hand side and solution pairs

Could we extend LinearVariationalSolver to support this?

[pbrubeck]

But why is LinearVariationalSolver not suitable for those cases? We can now represent assembled matrices in UFL so can't the code paths be unified? I think @JHopeCollins has had some success with this sort of thing using external operators (when we were discussing taping for LinearSolver).

There are a couple of cases where LinearVariationalSolver needs to catch up twith LinearSolver to support the more generic operators that LinearSolver is currently capable of handling. This is mostly to do with the fact that the action of the operator apears in the linear form RHS, and fieldsplit will struggle with this as the second argument in the action would no longer be a Function, but a vector of split Functions. This PR is to fix this.

Surely multiple methods to achieve the same result suggests that things should be combined?

We could allow LinearVariationalSolver.solve(x, b) to take in the solution-RHS pair as optional arguments. But having a class that also sets up both the LinearVariationalProblem and the solver is another argument to keep LinearSolver.

Could we extend LinearVariationalSolver to support this?

Basically the PR consists mostly of enhacements to LinearVariationalSolver.

[connorjward]

Basically the PR consists mostly of enhancements to LinearVariationalSolver.

Ah I see. Could we therefore think of this PR as a step along the way to fully removing LinearSolver (at a later date)? If so then I am completely on board.

[connorjward]

I think I have only noted small things.

Also, does this PR add any new functionality? If so it would be great to test it.

[Ig-dolci]

Description

Provides enhacements for LinearVariationalSolver to support more generic MatrixBase operators, and makes LinearSolver a wrapper for LinearVariationalSolver,

Just a note to pay attention for adjoints: I am more concerned about how the adjoint-based gradients will work with LinearVariationalSolver supporting MatrixBase. Currently, we have LinearVariationalSolver that works perfectly to have the adjoint equations and to obtain and solve an adjoint-based gradient form. Considering this additional feature, I do not see how adjoint-based gradient will work for general cases.

[pbrubeck]

Just a note to pay attention for adjoints: I am more concerned about how the adjoint-based gradients will work with LinearVariationalSolver supporting MatrixBase. Currently, we have LinearVariationalSolver that works perfectly to have the adjoint equations and to obtain and solve an adjoint-based gradient form. Considering this additional feature, I do not see how adjoint-based gradient will work for general cases.

@Ig-dolci We support variational problems defined purely in UFL, thus we get differentiability and adjoints. The enhacements are to do with Fieldsplit.