add gradient cache in Optimizer #66
Conversation
|
Closes #68 |
Codecov Report
@@ Coverage Diff @@
## master #66 +/- ##
==========================================
- Coverage 88.33% 85.23% -3.10%
==========================================
Files 4 4
Lines 300 359 +59
==========================================
+ Hits 265 306 +41
- Misses 35 53 +18
Continue to review full report at Codecov.
|
|
OSQP appears to have more inaccuracy / variations on the final result, that's why I switched to Ipopt |
Yes, when I use it for the tests, I usually increase the accuracy, e.g. "eps_abs" => 1e-8,
"eps_rel" => 1e-8, |
src/MOI_wrapper.jl
Outdated
| @@ -73,6 +73,7 @@ mutable struct Optimizer{OT <: MOI.ModelLike} <: MOI.AbstractOptimizer | |||
| dual_optimal::Vector{Union{Vector{Float64}, Float64}} # refer - https://github.com/jump-dev/DiffOpt.jl/issues/21#issuecomment-651130485 | |||
| var_idx::Vector{VI} | |||
| con_idx::Vector{CI} | |||
| gradient_cache::NamedTuple | |||
There was a problem hiding this comment.
Why not using Union{Nothing, NamedTuple{...}} or even Union{Nothing, Cache} with an actual struct ? Currently, it's type unstable
There was a problem hiding this comment.
This will need at least Union{Nothing, NamedTuple{T1}, NamedTuple{T2}} for the cache, since we have possible cache types depending on conic or not
There was a problem hiding this comment.
I'd still prefer struct Cache1 end; struct Cache2 end; Union{Nothing, Cache1, Cache2}, otherwise it's not so easy to follow what's going on.
Moreover, it seems to me that NamedTuple are used when you don't know a priori what is the struct so it's like creating new structs on-the-fly why here we know very well the struct at coding time so we should use structs as it's more readable.
|
bump here |
Fixes #61
still failing because of #65