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

Remove dictionaries tracking SingleVariable constraints #2032

Merged
merged 1 commit into from Aug 20, 2019

Conversation

@mlubin
Copy link
Member

commented Aug 19, 2019

This follows from JuliaOpt/MathOptInterface.jl#570.

Unfortunately the performance issue in #1607 is not resolved:

using JuMP
using BenchmarkTools

function plain(N)
    model = Model()
    x = @variable(model, [1:N])
    return x
end
@btime plain(1_000_000)

function with_int_and_bounds(N)
    model = Model()
    x = @variable(model, [1:N], Int, lower_bound=-1, upper_bound=1)
    return x
end
@btime with_int_and_bounds(1_000_000)

yields

  67.775 ms (2000220 allocations: 73.43 MiB)
  674.142 ms (18000220 allocations: 305.57 MiB)

So it's still about 10x slower to create variables with bounds than without them. I haven't started looking into the reasons for this.

src/variables.jl Outdated Show resolved Hide resolved
src/variables.jl Outdated Show resolved Hide resolved

@mlubin mlubin force-pushed the ml/singlevar branch from e29d901 to 3702054 Aug 20, 2019

@codecov

This comment has been minimized.

Copy link

commented Aug 20, 2019

Codecov Report

Merging #2032 into master will decrease coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2032      +/-   ##
==========================================
- Coverage   91.46%   91.41%   -0.06%     
==========================================
  Files          33       33              
  Lines        4217     4192      -25     
==========================================
- Hits         3857     3832      -25     
  Misses        360      360
Impacted Files Coverage Δ
src/copy.jl 95% <ø> (-1.3%) ⬇️
src/variables.jl 97.45% <100%> (-0.27%) ⬇️
src/JuMP.jl 84.09% <100%> (ø) ⬆️

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 d4454f7...3702054. Read the comment docs.

1 similar comment
@codecov

This comment has been minimized.

Copy link

commented Aug 20, 2019

Codecov Report

Merging #2032 into master will decrease coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2032      +/-   ##
==========================================
- Coverage   91.46%   91.41%   -0.06%     
==========================================
  Files          33       33              
  Lines        4217     4192      -25     
==========================================
- Hits         3857     3832      -25     
  Misses        360      360
Impacted Files Coverage Δ
src/copy.jl 95% <ø> (-1.3%) ⬇️
src/variables.jl 97.45% <100%> (-0.27%) ⬇️
src/JuMP.jl 84.09% <100%> (ø) ⬆️

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 d4454f7...3702054. Read the comment docs.

@mlubin

This comment has been minimized.

Copy link
Member Author

commented Aug 20, 2019

I did a big refactoring to avoid type instabilities due to accessing the backend. It made a big difference:

  100.329 ms (2000220 allocations: 126.46 MiB)
  291.198 ms (4000220 allocations: 154.98 MiB)

There are still two additional allocations per variable that are unaccounted for.

@blegat
blegat approved these changes Aug 20, 2019
Copy link
Member

left a comment

The allocations seem to come from setting upper and lower bounds (one for each) but when I directly call MOI.add_constraint to backend.model_like, it disappears and when I redefine MOI.add_constraint(::MOIU.CachingOptimizer, ...) (which the same code), it also disappears.
When change to code of MOI to remove everything in MOI.add_constraint(::MOIU.CachingOptimizer, ...) except the call to backend.model_cache we still have the allocation.
It seems very weird and I don't think there is anything we can do, it might be the compiler doing something suboptimal.

@mlubin mlubin merged commit be85238 into master Aug 20, 2019

1 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@mlubin mlubin deleted the ml/singlevar branch Aug 20, 2019

@blegat blegat added the breaking label Aug 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.