ds4dm/Tulip.jl

error `S` not defined

ericphanson opened this issue · 5 comments

Tulip looks great! I run into an error though, when trying out Convex#MathOptInterface's tests via its ProblemDepot. Running the script

using Pkg
Pkg.add(PackageSpec(name="Convex", url="https://github.com/ericphanson/Convex.jl", rev="MathOptInterface"))

using Convex, Tulip

Convex.ProblemDepot.run_tests(exclude=[r"sdp", r"socp", r"exp", r"mip"]) do p
    Convex.solve!(p, Tulip.Optimizer())
end

gives many errors such as

affine_negate_atom: Error During Test at /Users/eh540/.julia/packages/Convex/r4w0m/src/problem_depot/problem_depot.jl:72
  Got exception outside of a @test
  UndefVarError: S not defined
  Stacktrace:
   [1] add_constraint(::Tulip.Optimizer{Float64}, ::MathOptInterface.ScalarAffineFunction{Float64}, ::S<:MathOptInterface.EqualTo{Float64}) at /Users/eh540/.julia/dev/Tulip/src/MOI_wrapper.jl:758
   [2] add_constraint(::MathOptInterface.Utilities.CachingOptimizer{Tulip.Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}, ::MathOptInterface.ScalarAffineFunction{Float64}, ::MathOptInterface.EqualTo{Float64}) at /Users/eh540/.julia/packages/MathOptInterface/4hMCx/src/Utilities/cachingoptimizer.jl:250
...

The patch

diff --git a/src/MOI_wrapper.jl b/src/MOI_wrapper.jl
index 6dfaf40..675bc13 100644
--- a/src/MOI_wrapper.jl
+++ b/src/MOI_wrapper.jl
@@ -733,7 +733,7 @@ function MOI.add_constraint(
 ) where{Tv<:Real, S<:SCALAR_SETS{Tv}}
     # Check that constant term is zero
     if !iszero(f.constant)
-        throw(MOI.ScalarFunctionConstantNotZero{Tv, typeof(f), S}(f.constant))
+        throw(MOI.ScalarFunctionConstantNotZero{Tv, typeof(f), typeof(s)}(f.constant))
     end
 
     # Extract bound info
@@ -755,7 +755,7 @@ function MOI.add_constraint(
     cidx = add_constraint!(m.inner, "", lb, ub, colids, colvals)
 
     # Return MOI index
-    return MOI.ConstraintIndex{typeof(f), S}(cidx.uuid)
+    return MOI.ConstraintIndex{typeof(f), typeof(s)}(cidx.uuid)
 end

fixes the errors, and all the tests pass. However, I don't quite understand why the error occurs in the first place; it seems to me that S should be the same as typeof(s).

That's weird. I tried to run your script but did not run into any errors.

~/.julia/dev/Tulip$ julia --project=. --color=yes script.jl
  Updating registry at `~/.julia/registries/General`
  Updating git-repo `https://github.com/JuliaRegistries/General.git`
  Updating git-repo `https://github.com/ericphanson/Convex.jl`
 Resolving package versions...
  Updating `~/.julia/dev/Tulip/Project.toml`
 [no changes]
  Updating `~/.julia/dev/Tulip/Manifest.toml`
 [no changes]
Test Summary: | Pass  Total
constant      |   28     28
┌ Warning: Problem status INFEASIBLE; solution may be inaccurate.
└ @ Convex ~/.julia/packages/Convex/1aPP8/src/MOI_solve.jl:199
Test Summary: | Pass  Total
affine        |  139    139
Test Summary: | Pass  Total
lp            |   52     52

I tried this with both Julia 1.0.1 and 1.1.0. Package versions were Tulip#v0.1.0, and MOI#v0.9.2.
Can you see anything else that could cause the error?

Ah, I was trying on Julia 1.3-rc2 and MOI 0.9.3. I just tried with Julia 1.1 and MOI 0.9.3 and it worked. Sounds like a Julia 1.3 issue then. I can try with the latest nightly; maybe it's been fixed.

No, it doesn't work on the nightly. But it sounds like this is a base Julia bug, not a Tulip bug, so I'll close this.

Good to know.
I'm building on Julia nightly, which is now 1.4. I'll add Julia 1.3 to the build matrix and see if anything comes up.

Thanks for raising the issue.

Ok great. I ran Tulip's tests and they passed on 1.4, so I think they aren't hitting this particular problem. Actually, that's one thing this problem depot is for; allow easy end-to-end testing of problems for solvers, since sometimes the way problems get formulated from Convex yield more complicated problems than are usually tested (e.g. using Convex's tests yielded jump-dev/SCS.jl#153, jump-dev/MathOptInterface.jl#838, and jump-dev/SDPA.jl#17).

So once Convex#MathOptInterface is merged, it might be useful to add something like the script above to Tulip's tests.

By the way, I just realized I pushed something to that branch that accesses dual values without checking if the model supports it, and now I'm getting errors on Tulip from that. So I'll fix that :)