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

Moi v0.10 #220

Merged
merged 7 commits into from
Nov 17, 2021
Merged

Moi v0.10 #220

merged 7 commits into from
Nov 17, 2021

Conversation

Wikunia
Copy link
Member

@Wikunia Wikunia commented Oct 17, 2021

Currently there are still two problems:

  • I'm unsure how to use MOIT.runtests (maybe without the bride optimizer?)
  • I have a weird LowerBoundAlreadySet error in one feasibility pump problem. I'm not too familiar with the code by @blegat from removing the JuMP dependency on that. Will come back to it next Friday probably.

Currently testing involves:

] add JuMP#master  
add Cbc#master
add Ipopt#master

Not sure how to do compat only for test dependencies.

Closes #172

@ccoffrin
Copy link
Member

@odow any tips on the MOIT.runtests point?

@blegat
Copy link
Collaborator

blegat commented Oct 19, 2021

maybe without the bride optimizer?

Juniper supports everything so I don't know that the bridges would be used for:

MOI.supports_constraint(::Optimizer, ::Type{<:MOI.AbstractFunction}, ::Type{<:MOI.AbstractSet}) = true

@Wikunia
Copy link
Member Author

Wikunia commented Nov 13, 2021

Quite a lot of tests seem to be changed and break currently like getting a variable index by name. Kind of surprised that this should be handled by the solver itself.
Also I'm not a fan of returning the SolverVersion as I currently do because at one point in the future it will be definitely out of sync. Maybe there is a way to obtain the current version number though hopefully without an extra package dependency.

@blegat
Copy link
Collaborator

blegat commented Nov 13, 2021

For names, you could do

MOI.Test.Config(
    exclude = Any[
        MOI.ConstraintName,
        MOI.VariableName,
        MOI.Name,
    ],
)

@@ -15,11 +15,12 @@ end
MOI.is_valid(model::Optimizer, index::MOI.Index) = MOI.is_valid(model.model_cache, index)

MOI.get(::Optimizer, ::MOI.SolverName) = "Juniper"
MOI.get(::Optimizer, ::MOI.SolverVersion) = "v0.7.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can always not implement it and exclude the test.

test/MOI_wrapper.jl Outdated Show resolved Hide resolved
@odow
Copy link
Collaborator

odow commented Nov 13, 2021

Quite a lot of tests seem to be changed and break currently

To the contrary, because tests were opt-in, most solvers did not test against the full API of MathOptInterface, and missed implementing some critical features. The new approach is opt-out, which has exposed a lot of issues in the wrappers (many more than I expected!).

@Wikunia
Copy link
Member Author

Wikunia commented Nov 15, 2021

For names, you could do

MOI.Test.Config(
    exclude = Any[
        MOI.ConstraintName,
        MOI.VariableName,
        MOI.Name,
    ],
)

ConstraintName and VariableName is already excluded in there.
I've also tried to exclude ConstraintPrimal there but still getting errors regarding that.

@odow
Copy link
Collaborator

odow commented Nov 15, 2021

Just wrap it in a cachingoptimizer. It looks like not all the tests bail early if constraint name isn't supported.

@Wikunia
Copy link
Member Author

Wikunia commented Nov 15, 2021

that doesn't seem to work or am I doing something wrong?

@odow
Copy link
Collaborator

odow commented Nov 17, 2021

I'll take a look.

@odow odow mentioned this pull request Nov 17, 2021
@odow
Copy link
Collaborator

odow commented Nov 17, 2021

Now there is a single failure:

==================================
446
FP: Integer Test 2
447
==================================
448
FP: Integer test2: Error During Test at /home/runner/work/Juniper.jl/Juniper.jl/test/fpump.jl:123
449
  Got exception outside of a @test
450
  MathOptInterface.LowerBoundAlreadySet{MathOptInterface.EqualTo{Float64},MathOptInterface.EqualTo{Float64}}: Cannot add `VariableIndex`-in-`MathOptInterface.EqualTo{Float64}` constraint for variable MathOptInterface.VariableIndex(4) as a `VariableIndex`-in-`MathOptInterface.EqualTo{Float64}` constraint was already set for this variable and both constraints set a lower bound.
451
  Stacktrace:
452
   [1] add_constraint at /home/runner/.julia/packages/Ipopt/QF8Lc/src/MOI_wrapper.jl:812 [inlined]
453
   [2] add_constraint(::MathOptInterface.Bridges.LazyBridgeOptimizer{Ipopt.Optimizer}, ::MathOptInterface.VariableIndex, ::MathOptInterface.EqualTo{Float64}) at /home/runner/.julia/packages/MathOptInterface/jPhq9/src/Bridges/bridge_optimizer.jl:1546
454
   [3] _copy_constraints(::MathOptInterface.Bridges.LazyBridgeOptimizer{Ipopt.Optimizer}, ::Juniper.FixVariables{Float64,Juniper.IntegerRelaxation{Juniper.Optimizer}}, ::MathOptInterface.Utilities.IndexMap, ::Array{MathOptInterface.ConstraintIndex{MathOptInterface.VariableIndex,MathOptInterface.EqualTo{Float64}},1}) at /home/runner/.julia/packages/MathOptInterface/jPhq9/src/Utilities/copy.jl:241
455
   [4] _pass_constraints(::MathOptInterface.Bridges.LazyBridgeOptimizer{Ipopt.Optimizer}, ::Juniper.FixVariables{Float64,Juniper.IntegerRelaxation{Juniper.Optimizer}}, ::MathOptInterface.Utilities.IndexMap, ::Array{Any,1}) at /home/runner/.julia/packages/MathOptInterface/jPhq9/src/Utilities/copy.jl:296
456
   [5] default_copy_to(::MathOptInterface.Bridges.LazyBridgeOptimizer{Ipopt.Optimizer}, ::Juniper.FixVariables{Float64,Juniper.IntegerRelaxation{Juniper.Optimizer}}) at /home/runner/.julia/packages/MathOptInterface/jPhq9/src/Utilities/copy.jl:435
457
   [6] #copy_to#7 at /home/runner/.julia/packages/MathOptInterface/jPhq9/src/Bridges/bridge_optimizer.jl:421 [inlined]
458
   [7] copy_to(::MathOptInterface.Bridges.LazyBridgeOptimizer{Ipopt.Optimizer}, ::Juniper.FixVariables{Float64,Juniper.IntegerRelaxation{Juniper.Optimizer}}) at /home/runner/.julia/packages/MathOptInterface/jPhq9/src/Bridges/bridge_optimizer.jl:421
459
   [8] generate_real_nlp(::Juniper.Optimizer, ::Juniper.JuniperProblem, ::Array{Float64,1}; random_start::Bool) at /home/runner/work/Juniper.jl/Juniper.jl/src/fpump.jl:159
460
   [9] generate_real_nlp(::Juniper.Optimizer, ::Juniper.JuniperProblem, ::Array{Float64,1}) at /home/runner/work/Juniper.jl/Juniper.jl/src/fpump.jl:152
461
   [10] fpump(::Juniper.Optimizer, ::Juniper.JuniperProblem) at /home/runner/work/Juniper.jl/Juniper.jl/src/fpump.jl:351
462
   [11] optimize!(::Juniper.Optimizer) at /home/runner/work/Juniper.jl/Juniper.jl/src/MOI_wrapper/MOI_wrapper.jl:285
463
   [12] optimize! at /home/runner/.julia/packages/MathOptInterface/jPhq9/src/MathOptInterface.jl:81 [inlined]
464
   [13] optimize!(::MathOptInterface.Utilities.CachingOptimizer{Juniper.Optimizer,MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.GenericModel{Float64,MathOptInterface.Utilities.ObjectiveContainer{Float64},MathOptInterface.Utilities.VariablesContainer{Float64},MathOptInterface.Utilities.ModelFunctionConstraints{Float64}}}}) at /home/runner/.julia/packages/MathOptInterface/jPhq9/src/Utilities/cachingoptimizer.jl:285
465
   [14] optimize!(::MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Juniper.Optimizer,MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.GenericModel{Float64,MathOptInterface.Utilities.ObjectiveContainer{Float64},MathOptInterface.Utilities.VariablesContainer{Float64},MathOptInterface.Utilities.ModelFunctionConstraints{Float64}}}}}) at /home/runner/.julia/packages/MathOptInterface/jPhq9/src/Bridges/bridge_optimizer.jl:348
466
   [15] optimize! at /home/runner/.julia/packages/MathOptInterface/jPhq9/src/MathOptInterface.jl:81 [inlined]
467
   [16] optimize!(::MathOptInterface.Utilities.CachingOptimizer{MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Juniper.Optimizer,MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.GenericModel{Float64,MathOptInterface.Utilities.ObjectiveContainer{Float64},MathOptInterface.Utilities.VariablesContainer{Float64},MathOptInterface.Utilities.ModelFunctionConstraints{Float64}}}}},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.GenericModel{Float64,MathOptInterface.Utilities.ObjectiveContainer{Float64},MathOptInterface.Utilities.VariablesContainer{Float64},MathOptInterface.Utilities.ModelFunctionConstraints{Float64}}}}) at /home/runner/.julia/packages/MathOptInterface/jPhq9/src/Utilities/cachingoptimizer.jl:285
468
   [17] optimize!(::Model, ::Nothing; bridge_constraints::Bool, ignore_optimize_hook::Bool, kwargs::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}) at /home/runner/.julia/packages/JuMP/yP8fy/src/optimizer_interface.jl:195
469
   [18] optimize! at /home/runner/.julia/packages/JuMP/yP8fy/src/optimizer_interface.jl:167 [inlined] (repeats 2 times)
470
   [19] solve(::Model) at /home/runner/work/Juniper.jl/Juniper.jl/test/runtests.jl:52
471
   [20] top-level scope at /home/runner/work/Juniper.jl/Juniper.jl/test/fpump.jl:149
472
   [21] top-level scope at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1119
473
   [22] top-level scope at /home/runner/work/Juniper.jl/Juniper.jl/test/fpump.jl:124
474
   [23] top-level scope at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1119
475
   [24] top-level scope at /home/runner/work/Juniper.jl/Juniper.jl/test/fpump.jl:8
476
   [25] include(::String) at ./client.jl:457
477
   [26] top-level scope at /home/runner/work/Juniper.jl/Juniper.jl/test/runtests.jl:113
478
   [27] top-level scope at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1119
479
   [28] top-level scope at /home/runner/work/Juniper.jl/Juniper.jl/test/runtests.jl:108
480
   [29] include(::String) at ./client.jl:457
481
   [30] top-level scope at none:6
482
   [31] eval(::Module, ::Any) at ./boot.jl:347
483
   [32] exec_options(::Base.JLOptions) at ./client.jl:272
484
   [33] _start() at ./client.jl:506

@codecov
Copy link

codecov bot commented Nov 17, 2021

Codecov Report

Merging #220 (e875e2f) into master (df63a8d) will increase coverage by 0.24%.
The diff coverage is 91.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #220      +/-   ##
==========================================
+ Coverage   91.54%   91.78%   +0.24%     
==========================================
  Files          20       20              
  Lines        1951     1961      +10     
==========================================
+ Hits         1786     1800      +14     
+ Misses        165      161       -4     
Impacted Files Coverage Δ
src/Juniper.jl 100.00% <ø> (ø)
src/util.jl 73.61% <77.77%> (+1.22%) ⬆️
src/MOI_wrapper/MOI_wrapper.jl 84.73% <87.50%> (+0.16%) ⬆️
src/MOI_wrapper/results.jl 86.84% <100.00%> (+2.97%) ⬆️
src/bb_inits_and_defaults.jl 100.00% <100.00%> (ø)
src/filter.jl 100.00% <100.00%> (+2.85%) ⬆️
src/fpump.jl 89.58% <100.00%> (-0.13%) ⬇️
src/model.jl 96.15% <100.00%> (ø)
src/BnBTree.jl 94.25% <0.00%> (+0.30%) ⬆️
... and 2 more

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 df63a8d...e875e2f. Read the comment docs.

@odow
Copy link
Collaborator

odow commented Nov 17, 2021

@Wikunia have the feasibility pump tests always been flakey?

Linux:

image

Mac and Windows:

image

@Wikunia
Copy link
Member Author

Wikunia commented Nov 17, 2021

Thanks a lot @odow
I'm not sure about this particular test but some tests are unfortunately time dependent and then it can be the case that for whatever reason thae one system is slower and doesn't find the incumbent in time. I'm sure there are better ways to handle those things.

@odow
Copy link
Collaborator

odow commented Nov 17, 2021

Okay, fair enough. If/when I start working on nonlinear stuff I can take a longer look at some maintenance of Juniper. I've opened some issues to remind myself in the future.

This is good to go by me for the near term. Things are working and updated, at least.

@ccoffrin
Copy link
Member

Looks like tests are passing! Amazing, thanks @odow.

@Wikunia, would you like to merge and tag? I could also lend a hand with this if needed.

@Wikunia Wikunia merged commit 08d6de1 into master Nov 17, 2021
@odow odow deleted the MOI-v0.10 branch November 17, 2021 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support looking up variable/constraints by name
4 participants