Abstract linear solving method instead of \ #229
Conversation
Codecov ReportBase: 85.79% // Head: 86.34% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #229 +/- ##
==========================================
+ Coverage 85.79% 86.34% +0.54%
==========================================
Files 11 11
Lines 880 908 +28
==========================================
+ Hits 755 784 +29
+ Misses 125 124 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
|
||
| import DiffOpt | ||
| import LinearSolve | ||
| const LS = LinearSolve |
There was a problem hiding this comment.
Let's not introduce LS if we use it only 3 times, it just decreases readability IMO
|
|
||
| Q = view(LHS, 1:nv, 1:nv) | ||
| partial_grads = if norm(Q) ≈ 0 | ||
| -IterativeSolvers.lsqr(LHS', RHS) |
There was a problem hiding this comment.
This could also go through LinearSolve: https://github.com/SciML/LinearSolve.jl/blob/f2185983cfd4b25703c498975eb0174174bcc9e5/src/iterative_wrappers.jl#L55
|
As a user of DiffOpt, I confess I am not sure it's a good idea to integrate LinearSolve.jl (compared to, let's say, |
|
that's a fair point, we could readjust the design to just have an abstract attribute on top of which people can choose what solver to take |
|
Yes, 126 dependencies seems like a lot... https://juliahub.com/ui/Packages/LinearSolve/WR6RC/1.35.0?page=1 |
matbesancon
changed the title
|
following the discussion, I changed the design to offer an abstract interface, people can change the linear system solve without requiring LinearSolve, since we do not batch, use cache or anything |
|
Looks good, will be useful for the rank-1 example. Could you add a test for setting and getting the attribute ? |
Co-authored-by: Benoît Legat <[email protected]>
|
just did, should be good once CI passes |
|
I also added an |
| @test MOI.get(model, DiffOpt.QuadraticProgram.LinearAlgebraSolver()) === nothing | ||
| MOI.set(model, DiffOpt.QuadraticProgram.LinearAlgebraSolver(), TestSolver()) | ||
| @test MOI.get(model, DiffOpt.QuadraticProgram.LinearAlgebraSolver()) !== nothing | ||
| MOI.empty!(model) |
There was a problem hiding this comment.
If it is an optimizer attribute, it should not be reset by MOI.empty!
No description provided.