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

Update for JuMP v0.16 #74

Merged
merged 10 commits into from
Oct 20, 2017
Merged

Update for JuMP v0.16 #74

merged 10 commits into from
Oct 20, 2017

Conversation

yeesian
Copy link
Collaborator

@yeesian yeesian commented Mar 23, 2017

Main changes:

  • drop curly braces in comprehension syntax
  • JuMP no longer has a mechanism for selecting solvers by default. Not specifying a solver before calling solve() will result in an error.

@ShimShtern let us know if it fixes the problem(s) you've faced in https://discourse.julialang.org/t/issues-with-jumper/2794

@codecov-io
Copy link

codecov-io commented Mar 29, 2017

Codecov Report

Merging #74 into master will decrease coverage by 0.19%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #74     +/-   ##
=========================================
- Coverage   84.33%   84.13%   -0.2%     
=========================================
  Files          15       15             
  Lines        1334     1330      -4     
=========================================
- Hits         1125     1119      -6     
- Misses        209      211      +2
Impacted Files Coverage Δ
src/uncsets_basic_cut.jl 98.5% <100%> (-0.07%) ⬇️
src/solve.jl 91.02% <100%> (ø) ⬆️
src/JuMPeR.jl 83.87% <0%> (-3.23%) ⬇️
src/print.jl 86.14% <0%> (-0.38%) ⬇️
src/uncsets_basic_reform.jl 100% <0%> (ø) ⬆️

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 e161a36...8327d36. Read the comment docs.

yeesian added 2 commits March 28, 2017 23:52
it’s started to be a little dated
@yeesian
Copy link
Collaborator Author

yeesian commented Mar 29, 2017

@IainNZ sorry for the noise; this is now ready for review.

@@ -191,7 +190,7 @@ print_with_color(:yellow, " MILP tests...\n")
@objective(m, Max, 1.1*x[1] + x[2])
@constraint(m, u1*x[1] + 1*x[2] <= 2)
@constraint(m, u2*x[1] + 1*x[2] <= 6)
@test solve(m, prefer_cuts=cuts) == :Optimal
@test solve(m, disable_cuts=cuts) == :Optimal
Copy link
Owner

Choose a reason for hiding this comment

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

Whats going on here? Was this test not being run before?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry my bad, it used to fail on travis (error message here: https://travis-ci.org/IainNZ/JuMPeR.jl/builds/216164410#L1304) when it was passing locally, and I was trying to understand the difference between prefer_cuts and disable_cuts, but forgot to switch back.

end
end

@testset "Affine, manual" begin
rm = RobustModel()
rm = RobustModel(solver=GLPKSolverLP())
Copy link
Owner

Choose a reason for hiding this comment

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

shouldn't this be solver=solver or something like that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah my bad, it should

@@ -13,7 +13,7 @@
# "Adjustable Robust Solutions of Uncertain Linear Programs"
#-----------------------------------------------------------------------

using JuMP, JuMPeR
using JuMP, JuMPeR, GLPKMathProgInterface
Copy link
Owner

Choose a reason for hiding this comment

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

GLPK needed here?

@@ -20,7 +20,7 @@
# D ∈ { ‖(D - μ)/σ‖₁ ≤ ⌊√N⌋, ‖(D - μ)/σ‖∞ ≤ 1 }
#-----------------------------------------------------------------------

using JuMP, JuMPeR
using JuMP, JuMPeR, GLPKMathProgInterface
Copy link
Owner

Choose a reason for hiding this comment

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

GLPK needed here?

@@ -56,7 +56,7 @@ print_with_color(:yellow, "Adaptive Newsvendor Model...\n")
end

@testset "Static, manual" begin
m = RobustModel()
m = RobustModel(solver=GLPKSolverLP())
Copy link
Owner

Choose a reason for hiding this comment

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

Should be =solver?

test/uncsets.jl Outdated
@test_throws ErrorException JuMPeR.generate_reform(IncompleteSet(), RobustModel(), Int[])
@test_throws ErrorException JuMPeR.generate_cut(IncompleteSet(), RobustModel(), Int[])
@test_throws ErrorException JuMPeR.generate_scenario(IncompleteSet(), RobustModel(), Int[])
@test_throws ErrorException JuMPeR.setup_set(IncompleteSet(), RobustModel(solver=GLPKSolverLP()), Int[], false, nothing)
Copy link
Owner

Choose a reason for hiding this comment

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

Could be lp_solvers[0] then it works with any solver?

@IainNZ
Copy link
Owner

IainNZ commented Apr 5, 2017

Getting closer
Not sure why tests are failing?

@yeesian
Copy link
Collaborator Author

yeesian commented Apr 5, 2017

Re the following test:

@test string(new_con) == "5 x[1] + x[2] + 4 x[3] + 11 x[4] $(JuMP.repl[:leq]) 10"

It sometimes fails with

string(new_con) = "4.999999999999986 x[1] + x[2] + 3.9999999999999805 x[3] + 10.999999999999947 x[4] ≤ 9.999999999999975"

How do you feel about replacing it with a test like

@test all(contains(string(new_con), "x[$i]") for i in 1:4)
@test contains(string(new_con), "$(JuMP.repl[:leq])")

?

@yeesian
Copy link
Collaborator Author

yeesian commented Apr 5, 2017

Also,

isapprox(getvalue(F),44272.82749,atol=TOL)

sometimes fails when

getvalue(F) = 44272.823734690275

when using Ipopt. But passes with ECOS, Gurobi, Clp, and GLPK.

Okay to relax the tolerance?

yeesian added 3 commits April 5, 2017 18:24
and relax test on constraint expression printing
@yeesian
Copy link
Collaborator Author

yeesian commented Apr 5, 2017

Not sure why tests are failing?

Yeah, still failing on Travis for the same (as yet unknown) reasons.

@blegat
Copy link

blegat commented Oct 18, 2017

@yeesian Are tests passing now ? Github says that all checks are passing.
Related: jump-dev/JuMP.jl#1116

@yeesian
Copy link
Collaborator Author

yeesian commented Oct 19, 2017

Yeah, I believe so. @IainNZ good to merge?

@IainNZ
Copy link
Owner

IainNZ commented Oct 20, 2017 via email

@yeesian yeesian changed the title Update for current version of JuMP Update for JuMP v0.6 Oct 20, 2017
@yeesian yeesian merged commit 090ce1d into master Oct 20, 2017
@yeesian yeesian changed the title Update for JuMP v0.6 Update for JuMP v0.16 Oct 20, 2017
@yeesian yeesian deleted the jump015 branch September 26, 2018 01:56
yeesian added a commit that referenced this pull request Feb 23, 2019
it is correct, but the GLPK solver has been problematic on it. see
* #74 (comment)
and
*
8327d3648b0aa59f5355
f9bd56b65e8dda6fa642
for workarounds in the past.
yeesian added a commit that referenced this pull request Feb 23, 2019
* update for julia 1.0

* update travis script

* add LinearAlgebra to REQUIRE

* some updates

* more changes

* add constructors

* define constructor for numbers too

* don't broadcast over norm expressions

* revert to map

* switch back to working version of GLPK

* comment out flakey test for now

it is correct, but the GLPK solver has been problematic on it. see
* #74 (comment)
and
*
8327d3648b0aa59f5355
f9bd56b65e8dda6fa642
for workarounds in the past.

* test on different versions of julia
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.

4 participants