[WIP] Update polyhedron(::JuMP.Model) to Jump 0.19

[OliverEvans96]

Hi all,

Thanks for the great package! I've been working on updating the LP -> polyhedron conversion method to JuMP 0.19 (solves #152 in part). It seems to be working for the simple case I've dealt with, though I haven't yet written proper tests so far.

I'm additionally having a bit of a versioning issue, #154, which is preventing me from running tests properly.

Any feedback and suggestions are appreciated!

Cheers, Oliver

[blegat]

Thanks for the help !

I'm additionally having a bit of a versioning issue, #154, which is preventing me from running tests properly.

You need to update the line in the REQUIRE file to JuMP 0.19

[blegat]

I would recommend the following strategy:

@model(_MOIModel,
      (),                                                         # untyped scalar sets
      (MOI.EqualTo, MOI.LessThan,), #   typed scalar sets
      (),            # untyped vector sets
      (),                                                         #   typed vector sets
      (MOI.SingleVariable,),                                      # untyped scalar functions
      (MOI.ScalarAffineFunction,),                                #   typed scalar functions
      (),                                   # untyped vector functions
      ())                                #   typed vector functions

so that any constraint that is not in the form HyperPlane or HalfSpace is transformed into it by constraint bridges (instead of having to define a complicated iteration in LPHRepresentation as we had to do until now. We still need to leave SingleVariable until https://github.com/JuliaOpt/MathOptInterface.jl/pull/659 is merged. Then given a JuMP model, we can do

polyhedra_model = _MOIModel{Flaot64}()
bridged = MOI.Bridges.full_bridge_optimizer(polyhedra_model, Float64)
MOI.copy_to(bridged, backend(model), copy_names = false)

then in polyhedra_model you only have to deal with a fixed set of constraints that you can access with e.g. MOI.ListOfConstraintIndices{MOI.ScalarAffineFunction{Float64}, MOI.LessThan{Float64}}().

[OliverEvans96]

@blegat thanks for the detailed review!

Unfortunately, I don't have any idea what you're talking about in that comment.

I would recommend the following strategy: ...

Unfortunately, I don't have any idea what you're talking about in that comment. Why are we creating a model? Are you referring to creating a model from a polyhedron?

[blegat]

The LPHRepresentation defined here: https://github.com/JuliaPolyhedra/Polyhedra.jl/blob/fc51bb5efdb4270ef8ff71561bf07bedc63ece0d/src/lphrep.jl#L4-L17 Is rather complicated and its design inherits from the MathProgBase LPQP model. As a consequence, defining the halfspaces and hyperplanes iterators is rather tedious. If we defined the model as in https://github.com/JuliaPolyhedra/Polyhedra.jl/pull/155#issuecomment-471357070 we can simply do

struct LPHRep{T}
    model::_MOIModel{T}
end

The full dimension of p::LPHRep is simply MOI.get(p.model, MOI.NumberOfVariables()) and the number of halfspaces is simply MOI.get(p.model, MOI.NumberOfConstraints{MOI.ScalarAffineFunction{Float64}, MOI.LessThan{Float64}}()), and so on. With this representation of LPHRep, it is easy to build it form a JuMP model as detailed in https://github.com/JuliaPolyhedra/Polyhedra.jl/pull/155#issuecomment-471357070 and it is easy to defined halfspace and hyperplanes iterators as shown above

[OliverEvans96]

Ah, thanks for the explanation - that makes sense!

[blegat]

I created a PR implementing the approach I detailed here: https://github.com/JuliaPolyhedra/Polyhedra.jl/pull/157. The issue is a bit tricky and requires a good understanding of Polyhedra internals that I didn't sufficiently document, and I'd like to release it before going on holidays. Thank for your contribution, don't hesitate to propose other changes. Sorry for taking over, I should have communicated it more clearly, e.g. by assign myself to the issue, to avoid duplicate work.

[OliverEvans96]

No worries - I trust that your solution is superior to my hack! Thanks for making the update! Let me know if there's anything else related to this PR that I can do to help.

Oliver