Skip to content

add L_1-norm and L_infinity-norm cones and tests #818

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 30 commits into from
Aug 16, 2019
Merged

add L_1-norm and L_infinity-norm cones and tests #818

merged 30 commits into from
Aug 16, 2019

Conversation

chriscoey
Copy link
Contributor

@chriscoey chriscoey commented Aug 7, 2019 •
edited
Loading

The names can be bikeshed.
I plan to add bridges from these sets to LP constraints (edit: in a PR after this is merged) (edit 2: may as well add bridges here).
Hypatia supports these cones, and has significantly better performance with these cones than with the LP extended formulations.

@chriscoey
Copy link
Contributor Author

Note: the second test instance for both cones is the same as the SOC3 test, but with the 2-dim SOC replaced with equivalent 2-dim norm1 or norminf cones. The contconic file can do with quite a bit of refactoring and cleanup but that should be done separately.

@chriscoey
Copy link
Contributor Author

Failures for LazyBridgeOptimizer. I don't understand that part.

@blegat
Copy link
Member

blegat commented Aug 7, 2019

Failures for LazyBridgeOptimizer. I don't understand that part.

Just exclude the tests and add a comment to add them back once bridges are implemented

@codecov-io
Copy link

codecov-io commented Aug 7, 2019 •
edited
Loading

Codecov Report

Merging #818 into master will increase coverage by 0.09%.
The diff coverage is 98.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #818      +/-   ##
==========================================
+ Coverage   94.83%   94.93%   +0.09%     
==========================================
  Files          71       72       +1     
  Lines        7649     7872     +223     
==========================================
+ Hits         7254     7473     +219     
- Misses        395      399       +4
Impacted Files Coverage Δ
src/Utilities/model.jl 93.5% <ø> (ø) ⬆️
src/Test/UnitTests/basic_constraint_tests.jl 98% <ø> (ø) ⬆️
src/Bridges/Constraint/norm_to_lp.jl 100% <100%> (ø)
src/Utilities/functions.jl 94.63% <100%> (ø) ⬆️
src/Bridges/Constraint/Constraint.jl 100% <100%> (ø) ⬆️
src/sets.jl 94.73% <100%> (+0.29%) ⬆️
src/Test/contconic.jl 99.42% <97.05%> (-0.36%) ⬇️

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 40aa3e5...6e6d5db. Read the comment docs.

chriscoey
chriscoey commented Aug 8, 2019
View reviewed changes
chriscoey
chriscoey commented Aug 8, 2019
View reviewed changes
chriscoey
chriscoey commented Aug 8, 2019
View reviewed changes
@chriscoey
Copy link
Contributor Author

Bridges added, NEWS updated, tests passing. Please review

blegat
blegat reviewed Aug 8, 2019
View reviewed changes
blegat
blegat reviewed Aug 8, 2019
View reviewed changes
blegat
blegat reviewed Aug 8, 2019
View reviewed changes
blegat
blegat reviewed Aug 8, 2019
View reviewed changes
@chriscoey
Copy link
Contributor Author

@blegat I addressed your comments except for #818 (comment)

chriscoey
chriscoey commented Aug 8, 2019
View reviewed changes
chriscoey
chriscoey commented Aug 8, 2019
View reviewed changes
chriscoey
chriscoey commented Aug 8, 2019
View reviewed changes
blegat
blegat reviewed Aug 8, 2019
View reviewed changes
@chriscoey
Copy link
Contributor Author

chriscoey commented Aug 9, 2019 •
edited
Loading

@blegat should I have a go at implementing constraint duals thru the bridge? any expected gotchas there?

edit: I'm not sure whether allowing VectorQuadraticFunction would complicate that. and if it does complicate it, I think having access to duals through the bridge is much more important than being able to use VQF through bridges with these cones.

edit 2: and another reason I now oppose supporting VQF is that the bridge could convert a convex model to a nonconvex model (unless we take significantly more care and use DCP rules, which I'm not prepared to do at this stage).

@blegat
Copy link
Member

blegat commented Aug 9, 2019

I'm not sure whether allowing VectorQuadraticFunction would complicate that

I don't think so, just implement duals thinking about the VectorAffineFunction and it would automatically work also for VQF because what you do is basically multiplying by the adjoint of the linear map between the two sets and you can proof that it's correct in the lagrangian function so it should work with any function so using AbstractVectorFunction is correct. See slide 6 of https://www.researchgate.net/publication/334574258_Automatic_reformulation_using_optimization_bridges

that the bridge could convert a convex model to a nonconvex model

Why is that ? If the quadratic function x_i have a PSD Q then t >= -x_i is indeed nonconvex but wasn't the initial model also nonconvex ?

chriscoey
chriscoey commented Aug 14, 2019
View reviewed changes
chriscoey
chriscoey commented Aug 14, 2019
View reviewed changes
@chriscoey
Copy link
Contributor Author

@blegat I need your help to finish this. See my comment 913e3db#r314057544.
Beyond that, there are a couple of failing "basic constraint tests" that I can't figure out.

@chriscoey
Copy link
Contributor Author

@blegat thanks for the fix in #837.
Basic constraint tests are now failing later, and I'll need your help again.
There seem to be 3 types of failure/error (same for NormOne and NormInfinity):

julia> @testset begin include("test/Bridges/Constraint/norm_to_lp.jl") end

ConstraintFunction: Error During Test at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:210
  Test threw exception
  Expression: func ≈ constraint_function
  MethodError: no method matching isapprox(::MathOptInterface.VectorAffineFunction{Float64}, ::MathOptInterface.VectorOfVariables)
  Closest candidates are:
    isapprox(!Matched::Missing, ::Any; kwargs...) at missing.jl:69
    isapprox(::Any, !Matched::Missing; kwargs...) at missing.jl:70
    isapprox(!Matched::Union{MathOptInterface.SingleVariable, MathOptInterface.VectorOfVariables}, ::Union{MathOptInterface.SingleVariable, MathOptInterface.VectorOfVariables}; kwargs...) at /home/coey/.julia/dev/MathOptInterface/src/functions.jl:281
    ...
  Stacktrace:
   [1] eval_test(::Expr, ::Expr, ::LineNumberNode, ::Bool) at /home/coey/julia/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:244
   [2] macro expansion at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:210 [inlined]
   [3] macro expansion at /home/coey/julia/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1114 [inlined]
   [4] macro expansion at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:209 [inlined]
   [5] macro expansion at /home/coey/julia/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1114 [inlined]
   [6] #basic_constraint_test_helper#39(::Bool, ::Bool, ::Bool, ::typeof(MathOptInterface.Test.basic_constraint_test_helper), ::MathOptInterface.Bridges.Constraint.SingleBridgeOptimizer{MathOptInterface.Bridges.Constraint.NormInfinityBridge{Float64,F} where F,MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.Model{Float64}}}, ::MathOptInterface.Test.TestConfig, ::getfield(MathOptInterface.Test, Symbol("##10#11")), ::MathOptInterface.NormInfinityCone, ::Int64) at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:196
  
ConstraintFunction: Test Failed at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:211
  Expression: variables(func) == variables(constraint_function)
   Evaluated: Set(MathOptInterface.VariableIndex[MathOptInterface.VariableIndex(12345677), MathOptInterface.VariableIndex(12345676), MathOptInterface.VariableIndex(12345679)]) == MathOptInterface.VariableIndex[MathOptInterface.VariableIndex(12345679), MathOptInterface.VariableIndex(12345676), MathOptInterface.VariableIndex(12345677)]
Stacktrace:
 [1] macro expansion at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:211 [inlined]
 [2] macro expansion at /home/coey/julia/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1114 [inlined]
 [3] macro expansion at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:209 [inlined]
 [4] macro expansion at /home/coey/julia/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1114 [inlined]
 [5] #basic_constraint_test_helper#39(::Bool, ::Bool, ::Bool, ::typeof(MathOptInterface.Test.basic_constraint_test_helper), ::MathOptInterface.Bridges.Constraint.SingleBridgeOptimizer{MathOptInterface.Bridges.Constraint.NormInfinityBridge{Float64,F} where F,MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.Model{Float64}}}, ::MathOptInterface.Test.TestConfig, ::getfield(MathOptInterface.Test, Symbol("##10#11")), ::MathOptInterface.NormInfinityCone, ::Int64) at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:196

ConstraintFunction: Error During Test at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:208
  Got exception outside of a @test
  MethodError: no method matching MathOptInterface.VectorAffineTerm(::Int64, ::MathOptInterface.ScalarQuadraticTerm{Float64})
  Closest candidates are:
    MathOptInterface.VectorAffineTerm(::Int64, !Matched::MathOptInterface.ScalarAffineTerm{T}) where T at /home/coey/.julia/dev/MathOptInterface/src/functions.jl:103
    MathOptInterface.VectorAffineTerm(::Integer, !Matched::MathOptInterface.ScalarAffineTerm) at /home/coey/.julia/dev/MathOptInterface/src/functions.jl:108
  Stacktrace:
   [1] unsafe_add(::MathOptInterface.VectorQuadraticTerm{Float64}, ::MathOptInterface.VectorQuadraticTerm{Float64}) at /home/coey/.julia/dev/MathOptInterface/src/Utilities/functions.jl:331
   [2] sort_and_compress!(::Array{MathOptInterface.VectorQuadraticTerm{Float64},1}, ::typeof(MathOptInterface.term_indices), ::getfield(MathOptInterface.Utilities, Symbol("##24#26")), ::typeof(MathOptInterface.Utilities.unsafe_add)) at /home/coey/.julia/dev/MathOptInterface/src/Utilities/functions.jl:427
   [3] canonicalize! at /home/coey/.julia/dev/MathOptInterface/src/Utilities/functions.jl:408 [inlined]
   [4] get(::MathOptInterface.Bridges.Constraint.SingleBridgeOptimizer{MathOptInterface.Bridges.Constraint.NormInfinityBridge{Float64,F} where F,MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.Model{Float64}}}, ::MathOptInterface.ConstraintFunction, ::MathOptInterface.Bridges.Constraint.NormInfinityBridge{Float64,MathOptInterface.VectorQuadraticFunction{Float64}}) at /home/coey/.julia/dev/MathOptInterface/src/Bridges/Constraint/norm_to_lp.jl:49
   [5] get(::MathOptInterface.Bridges.Constraint.SingleBridgeOptimizer{MathOptInterface.Bridges.Constraint.NormInfinityBridge{Float64,F} where F,MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.Model{Float64}}}, ::MathOptInterface.ConstraintFunction, ::MathOptInterface.ConstraintIndex{MathOptInterface.VectorQuadraticFunction{Float64},MathOptInterface.NormInfinityCone}) at /home/coey/.julia/dev/MathOptInterface/src/Bridges/bridge_optimizer.jl:515
   [6] macro expansion at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:209 [inlined] (repeats 2 times)
   [7] macro expansion at /home/coey/julia/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1114 [inlined]
   [8] #basic_constraint_test_helper#39(::Bool, ::Bool, ::Bool, ::typeof(MathOptInterface.Test.basic_constraint_test_helper), ::MathOptInterface.Bridges.Constraint.SingleBridgeOptimizer{MathOptInterface.Bridges.Constraint.NormInfinityBridge{Float64,F} where F,MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.Model{Float64}}}, ::MathOptInterface.Test.TestConfig, ::getfield(MathOptInterface.Test, Symbol("##18#19")), ::MathOptInterface.NormInfinityCone, ::Int64) at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:196
   [9] (::getfield(MathOptInterface.Test, Symbol("#kw##basic_constraint_test_helper")))(::NamedTuple{(:delete, :get_constraint_function, :get_constraint_set),Tuple{Bool,Bool,Bool}}, ::typeof(MathOptInterface.Test.basic_constraint_test_helper), ::MathOptInterface.Bridges.Constraint.SingleBridgeOptimizer{MathOptInterface.Bridges.Constraint.NormInfinityBridge{Float64,F} where F,MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.Model{Float64}}}, ::MathOptInterface.Test.TestConfig, ::Function, ::MathOptInterface.NormInfinityCone, ::Int64) at ./none:0
   [10] macro expansion at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:131 [inlined]
   [11] macro expansion at /home/coey/julia/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1114 [inlined]
   [12] #basic_constraint_tests#20(::Bool, ::Bool, ::Bool, ::Array{Tuple{DataType,DataType},1}, ::Array{Any,1}, ::typeof(MathOptInterface.Test.basic_constraint_tests), ::MathOptInterface.Bridges.Constraint.SingleBridgeOptimizer{MathOptInterface.Bridges.Constraint.NormInfinityBridge{Float64,F} where F,MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.Model{Float64}}}, ::MathOptInterface.Test.TestConfig) at /home/coey/.julia/dev/MathOptInterface/src/Test/UnitTests/basic_constraint_tests.jl:130
   [13] (::getfield(MathOptInterface.Test, Symbol("#kw##basic_constraint_tests")))(::NamedTuple{(:include,),Tuple{Array{Tuple{DataType,DataType},1}}}, ::typeof(MathOptInterface.Test.basic_constraint_tests), ::MathOptInterface.Bridges.Constraint.SingleBridgeOptimizer{MathOptInterface.Bridges.Constraint.NormInfinityBridge{Float64,F} where F,MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.Model{Float64}}}, ::MathOptInterface.Test.TestConfig) at ./none:0
   [14] top-level scope at /home/coey/.julia/dev/MathOptInterface/test/Bridges/Constraint/norm_to_lp.jl:17
   [15] top-level scope at /home/coey/julia/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1114
   [16] top-level scope at /home/coey/.julia/dev/MathOptInterface/test/Bridges/Constraint/norm_to_lp.jl:15
   [17] include at ./boot.jl:328 [inlined]
   [18] include_relative(::Module, ::String) at ./loading.jl:1094
   [19] include(::Module, ::String) at ./Base.jl:31
   [20] include(::String) at ./client.jl:432
   [21] top-level scope at REPL[3]:1
   [22] top-level scope at /home/coey/julia/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1114
   [23] top-level scope at REPL[3]:1
   [24] eval(::Module, ::Any) at ./boot.jl:330
   [25] eval_user_input(::Any, ::REPL.REPLBackend) at /home/coey/julia/usr/share/julia/stdlib/v1.3/REPL/src/REPL.jl:86
   [26] macro expansion at /home/coey/julia/usr/share/julia/stdlib/v1.3/REPL/src/REPL.jl:118 [inlined]
   [27] (::getfield(REPL, Symbol("##26#27")){REPL.REPLBackend})() at ./task.jl:333

@chriscoey chriscoey mentioned this pull request Aug 15, 2019
Merged
@blegat
Copy link
Member

blegat commented Aug 15, 2019

You need to convert the VAF back into a VOV as done in:
https://github.com/JuliaOpt/MathOptInterface.jl/blob/0ed068ff8342cf2501305963c22708b0b18a4b80/src/Bridges/Constraint/rsoc.jl#L84

@chriscoey
Copy link
Contributor Author

That fixed one of the test failures. Then I made two small modifications to MOIU.functions.jl to get the new tests passing.

@chriscoey chriscoey requested a review from blegat August 15, 2019 19:12
@chriscoey
Copy link
Contributor Author

@mlubin @blegat please review

blegat
blegat reviewed Aug 15, 2019
View reviewed changes
@chriscoey
Copy link
Contributor Author

@blegat please merge if you're satisfied

@blegat blegat mentioned this pull request Aug 16, 2019
Merged
@chriscoey
Copy link
Contributor Author

tests passing. good to merge?

@blegat blegat merged commit 3f48578 into jump-dev:master Aug 16, 2019
@chriscoey chriscoey deleted the addL1Linf branch August 16, 2019 19:51
@chriscoey chriscoey mentioned this pull request Aug 16, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blegat blegat blegat approved these changes

@mlubin mlubin Awaiting requested review from mlubin

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chriscoey @blegat @codecov-io @mlubin