Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add documentation on performance and work-around for sparse variable creation #2126

Merged
merged 6 commits into from
Dec 29, 2019

Conversation

hellemo
Copy link
Contributor

@hellemo hellemo commented Dec 28, 2019

Add note on possible performance implications and suggested work-around following the discussion here:
https://discourse.julialang.org/t/working-efficiently-with-sparse-variables-in-jump/32240

@@ -387,6 +387,12 @@ JuMP.Containers.SparseAxisArray{VariableRef,1,Tuple{Int64}} with 2 entries:
[2] = x[2]
```

Note that with many indices and large amount of sparcity, variable construction may take more time than expected, as JuMP is effectively iterating over all indices and evaluating the conditional for each combination. The recommended work-around in such cases is to work directly with a list of tuples. For example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sparcity -> sparsity


```jldoctest; setup=:(model=Model())
S = [(1,1,1),(10,10,10)]
@variable(model, x[S])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make an example with both approach and then show that e.g. one has quadratic complexity and the other one has linear complexity? For instance with a condition i == j and n elements for i and j, it will be quadratic complexity vs linear complexity

@@ -387,6 +387,12 @@ JuMP.Containers.SparseAxisArray{VariableRef,1,Tuple{Int64}} with 2 entries:
[2] = x[2]
```

Note that with many indices and large amount of sparcity, variable construction may take more time than expected, as JuMP is effectively iterating over all indices and evaluating the conditional for each combination. The recommended work-around in such cases is to work directly with a list of tuples. For example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow the 80-character limit used in the rest of the documentation.

@@ -387,6 +387,12 @@ JuMP.Containers.SparseAxisArray{VariableRef,1,Tuple{Int64}} with 2 entries:
[2] = x[2]
```

Note that with many indices and large amount of sparcity, variable construction may take more time than expected, as JuMP is effectively iterating over all indices and evaluating the conditional for each combination. The recommended work-around in such cases is to work directly with a list of tuples. For example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"is effectively iterating" is unnecessarily imprecise, just "iterates"

@codecov
Copy link

codecov bot commented Dec 28, 2019

Codecov Report

Merging #2126 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2126   +/-   ##
=======================================
  Coverage   91.08%   91.08%           
=======================================
  Files          41       41           
  Lines        4376     4376           
=======================================
  Hits         3986     3986           
  Misses        390      390
Impacted Files Coverage Δ
src/JuMP.jl 80.98% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e52aa76...fcc93f6. Read the comment docs.

@mlubin
Copy link
Member

mlubin commented Dec 29, 2019

I don't want to merge this until we have a green light on the documentation build.

@blegat
Copy link
Member

blegat commented Dec 29, 2019

@hellemo can you rebase against master ?

@mlubin mlubin merged commit 04546c9 into jump-dev:master Dec 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants