Support sparsity #41
Support sparsity #41
Conversation
|
tested differentiating simple LP - https://github.com/AKS1996/jump-gsoc-2020/blob/master/DiffOpt_time_benchmarking.ipynb just adding support as basic SparseArrays reduced memory usage to a quarter BenchmarkTools.Trial:
memory estimate: 38.50 MiB
allocs estimate: 229521
--------------
minimum time: 78.738 ms (2.75% GC)
median time: 83.476 ms (5.46% GC)
mean time: 84.282 ms (4.85% GC)
maximum time: 94.965 ms (2.16% GC)
--------------
samples: 60
evals/sample: 1 |
| Q = [ | ||
| # reshaping vectors to matrix | ||
| b = b[:, :] | ||
| c = c[:, :] |
There was a problem hiding this comment.
why do we need these two lines?
There was a problem hiding this comment.
reindex/convert SparseVector to SparseMatrix. turns out SparseVector's give error on multiplication
src/MOI_wrapper.jl
Outdated
| @@ -529,7 +531,7 @@ For more info, refer https://github.com/matbesancon/MathOptSetDistances.jl | |||
| Find gradient of projection of vectors in `v` on product of `cones`. | |||
| For more info, refer https://github.com/matbesancon/MathOptSetDistances.jl | |||
| """ | |||
| Dπ(cones, v) = MOSD.projection_gradient_on_set(MOSD.DefaultDistance(), v, cones) | |||
| Dπ(cones, v) = sparse(MOSD.projection_gradient_on_set(MOSD.DefaultDistance(), v, cones)) | |||
There was a problem hiding this comment.
not sure if we force sparsity here or where Dπ is used
There was a problem hiding this comment.
yep. i just wanted to test how much we can improve on benchmarking. projections aren't that sparse, and ideally MOSD should return sparse values
There was a problem hiding this comment.
forcing sparsity of the type if the projection itself isn't sparse will result in poor performance
There was a problem hiding this comment.
ok, removing it from here
src/MOI_wrapper.jl
Outdated
|
|
||
| RHS = dQ * πz | ||
| if norm(RHS) <= 1e-4 | ||
| dz = zeros(Float64, size(RHS)) | ||
| dz = sparse(0.0I, size(RHS[:, :])) |
There was a problem hiding this comment.
yep, again RHS turns out to be a SparseVector
|
i know i've added |
| spzeros(n,n) dA' dc; | ||
| -dA spzeros(m,m) db; | ||
| -dc' -db' spzeros(1,1) | ||
| ]) |
There was a problem hiding this comment.
this part is doing a copy for nothing
if:
[
a b c
a b c
]
creates an already sparse matrix, then sparse(result) will do a copy for nothing. If the result is not sparse, a dense array is built out of the sparse arrays, which is then re-converted to sparse.
It would be better to create an empty dQ with spzeros(n+m+1, n+m+1) and then fill the non-diagonal blocks
with
dQ[n+1:n+1+m,1:n] .= -dA
etc.
Same thing above for the creation of Q
There was a problem hiding this comment.
oh. thanks for pointing out. never realized this redundant sparse-dense-sparse conversion
There was a problem hiding this comment.
I think the spzeros(1,1) can be replaced by just 0
|
@Aks1996 why close this one? |
|
there's a lot of code that's outdated (removed coz of SCS or moved to MatOI). it'll be suitable to add sparsity support to MatOI too. Once DiffOpt builds w/o errors, I'll do memory benchmarking and open separate PRs to support sparsity |
|
if you want, we can open this one too |
in light of #31