Add optional preconditioner for linear solves #279
Conversation
There was a problem hiding this comment.
Hi @mpf ! Thanks for the PR this a great addition. I made a few comments.
Two more that are not in the code:
- [ ] Can you add the new keyword argument L.18-29 in the docstring of the function
trunk? - [ ] Would you also have a unit test for this?
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
This doesn't seem to be a great preconditioner, but it does test the precon option.
|
@amontoison This isn't a great preconditioner, but it does test the new optional preconditioner argument. |
It's fine @mpf. The goal is just to ensure that we can provide a preconditioner without errors. |
|
A release would be great, and then I can let go of my fork. Thanks, @amontoison ! |
| function DiagPrecon(x) | ||
| H = Matrix(hess(nlp, x)) | ||
| λmin = minimum(eigvals(H)) | ||
| Diagonal(H + λmin * I) |
There was a problem hiding this comment.
Just so users aren't confused, we should add a little more than λmin here. If H has a negative eigenvalue, this preconditioner is singular.
| M = DiagPrecon(x0) | ||
| function LinearAlgebra.ldiv!(y, M::Diagonal, x) | ||
| y .= M \ x | ||
| end |
There was a problem hiding this comment.
This is already part of LinearAlgebra.
This PR allows for an optional preconditioner to be passed to the linear subsolver.