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

WIP: SDP and Block structure #156

Closed
wants to merge 4 commits into from
Closed

WIP: SDP and Block structure #156

wants to merge 4 commits into from

Conversation

joehuchette
Copy link
Contributor

This pull request adds SDP functionality to JuMP. Primal, dual, and matrix constraints can be used in modeling. Currently only Mosek support is available, although that will change soon. Left remaining before a merge:

  • Extensive tests on types, operators, etc.
  • Construct useful example suite
  • Documentation
  • Macros for setObjective, addConstraint
  • Absolute values (Absolute values in objective/constraints #48)
  • Handle MatrixExpr/NormExpr in setObjective
  • matrix operations on MatrixVar/MatrixExpr's

I'm still not quite sure what functionality will be most useful in practice (for instance, converting Lyapanov equations seems like it would be quite handy), so this should probably be considered an experimental feature for a release cycle or two, at the very least.

@mlubin
Copy link
Member

mlubin commented Apr 3, 2014

We're going to run out of features to add to JuMP soon.

@joehuchette
Copy link
Contributor Author

Not a bad problem to have.


SDPData() = SDPData(MatrixVar[], {}, {}, String[], MatrixConstraint[],PrimalConstraint[],DualConstraint[],MatrixFuncExpr(),{})

function SDPModel(;solver=MosekSolver())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this permanent (having a separate constructor)? Or will it eventually just be like nonlinear models?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason not to make it the same, I guess.

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 4, 2014

Very cool Joey, very very cool. Once you have an example in there I'll try and code up some of my own and can help with other aspects of it.

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 7, 2014

Didn't get very far :(

julia> using JuMP

julia> m=Model()
Feasibility problem with:
 * 0 linear constraints
 * 0 variables
Solver set to GLPK

julia> n = 4
4

julia> @defSDPVar(m, X[n])
ERROR: n not defined

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 7, 2014

then

julia> @defSDPVar(m, X[4])

julia> X
Evaluation succeeded, but an error occurred while showing value of type MatrixVar:
ERROR: m not defined
 in show at /home/idunning/.julia/v0.3/JuMP/src/sdp.jl:109
 in anonymous at show.jl:1042
 in showlimited at show.jl:1041
 in display at multimedia.jl:118
 in display at multimedia.jl:150

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 7, 2014

OK fixed second one

@joehuchette
Copy link
Contributor Author

Thanks, I think I just broke the first by accident a few commits ago.

@joehuchette
Copy link
Contributor Author

*Putting macro hat on"

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 7, 2014

OK, I was about to fix it, but I'll let you. Just need to escape I think

@mlubin
Copy link
Member

mlubin commented Apr 7, 2014

Relatedly, some tests should be added to hygiene.jl for the new macros.

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 7, 2014

We should bump Mosek at some point too, those deprecation warnings on released version are a pain.
New error:

idunning@IAIN-DESKTOP:~/.../JuMP/examples$ julia maxcut_sdp.jl 
ERROR: setupSDPVar! not defined
 in solveSDP at /home/idunning/.julia/v0.3/JuMP/src/sdp_solve.jl:184

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 7, 2014

Ah I see the "real" function name doesn't have a !

@joehuchette
Copy link
Contributor Author

Haha yeah, you caught me in the midst of some refactoring.

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 7, 2014

No problem, will just change locally

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 7, 2014

Hah maybe I should try later

ERROR: dim not defined
 in setupSDPVar at /home/idunning/.julia/v0.3/JuMP/src/sdp_solve.jl:138
 in solveSDP at /home/idunning/.julia/v0.3/JuMP/src/sdp_solve.jl:184

EDIT:
Making it

var.dim

seemed to work, actually solved it!

@joehuchette
Copy link
Contributor Author

Hahaha yeah, sorry, I've been pushing broken code today trying to get SDP bounds working properly. Should've branched on my branch...

@joehuchette
Copy link
Contributor Author

I'll let you know when things are fixed up later today.

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 7, 2014

Alright, my contribution: the Goemans-Williamson algorithm for approximating MAXCUT.
Unfortunately doesn't have any interesting contraints...

@joehuchette
Copy link
Contributor Author

Very cool. I'm hoping to find a practical example that mixes in different forms of constraints (primal, dual, etc), but nothing has come to me yet...

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 7, 2014

Would be nice to define

getValue(X[1,3])
ERROR: no method getValue(MatrixFuncVar)

@joehuchette
Copy link
Contributor Author

Yes, definitely need that

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 7, 2014

Changing objective sense/function after solving once doesn't seem to work (although it looks like it should at glance, so not sure whats going on). I've committed another example anyway - it fails because of this though.

@joehuchette
Copy link
Contributor Author

That's very strange, the sense is definitely getting communicated through to Mosek. Any case, thanks for the other nice example!

@mlubin
Copy link
Member

mlubin commented Apr 30, 2014

Just added an example, worked on the first try!

@joehuchette
Copy link
Contributor Author

Haha always a good sign, thanks!

add missing field in Model constructor

change behavior for x'*Q*x

minor fixes

work towards fixing tests
@coveralls
Copy link

Coverage Status

Coverage decreased (-4.15%) when pulling b179e0e on sdp into 7197c7d on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.93%) when pulling 4a9a50c on sdp into 7197c7d on master.

@joehuchette
Copy link
Contributor Author

@mlubin @IainNZ any idea how to debug module loading speed? This branch is painfully slow compared to master.

@mlubin
Copy link
Member

mlubin commented Jul 7, 2014

Given that this more than doubles the size of the JuMP source, would it make sense/be feasible to perhaps keep this in a separate package until Julia gets fast module loading?

@joehuchette
Copy link
Contributor Author

It would take a bit of work, but might be feasible. For instance, things like [x] are no longer valid in the operator code, since Base.vcat(::Variable) gets overwritten. There's some stuff in here that I would really like in JuMP proper, as well.

@mlubin
Copy link
Member

mlubin commented Jul 7, 2014

What stuff, specifically? If it's a relatively small chunk we can definitely merge it and put the bulk of the SDP code in a separate package.

@joehuchette
Copy link
Contributor Author

The norm stuff and x'*Q*x support would be great to have in master; everything that is not really tied to SDP, essentially. The problem is that as it's currently written, it's not possible to quarantine this support (which is probably a design flaw).

@IainNZ
Copy link
Collaborator

IainNZ commented Jul 7, 2014

Is it more than doubling startup time?

@joehuchette
Copy link
Contributor Author

Yeah, roughly.

@IainNZ
Copy link
Collaborator

IainNZ commented Jul 7, 2014

Interesting. If it was like, 4x startup time for 2x code I would have been hopeful that we could pull something off.

Can we do a conditional include when someone wants to use SDP? It'd be ugly... but its an ugly situation.

@joehuchette
Copy link
Contributor Author

Without a mechanism to pass arguments to using JuMP, I'm not quite sure how to do that. You're probably right, though; it makes sense that doubling the code base will double the load time.

@IainNZ
Copy link
Collaborator

IainNZ commented Jul 8, 2014

I was thinkng like

@load_JuMP_with_SDP()

or something wacky like that that would maybe do

using JuMP
include(Pkg.dir("JuMP") * "sdp.jl")

or whatever

@joehuchette joehuchette removed this from the 0.6 milestone Jul 24, 2014
@joehuchette
Copy link
Contributor Author

I rebased this locally and I have JuMP load time going from ~4 sec on master to ~5 sec on SDP. I think that seems reasonable to me, so maybe 0.6 (or immediately after 0.6 lands) is an acceptable target for this PR.

@joehuchette joehuchette added this to the 0.7 milestone Aug 15, 2014
@mlubin
Copy link
Member

mlubin commented Aug 15, 2014

Not bad at all. I'd prefer to merge this after 0.6.

@joehuchette
Copy link
Contributor Author

I agree. I think right after 0.6 would be ideal to try to work out as many kinks as possible before the next major release.

@mlubin
Copy link
Member

mlubin commented Dec 11, 2014

I guess we can untag this for 0.7?

@joehuchette
Copy link
Contributor Author

Yep.

@joehuchette joehuchette removed this from the 0.7 milestone Dec 11, 2014
@joehuchette joehuchette closed this Jun 9, 2015
@IainNZ
Copy link
Collaborator

IainNZ commented Jun 9, 2015

🌟 sdp branch 1️⃣ gone but never forgotten 🌟

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants