convexengineering / gpkit

Geometric programming for engineers
http://gpkit.readthedocs.org
MIT License
206 stars 40 forks source link

support for mosek 9 #1452

Closed rileyjmurray closed 4 years ago

rileyjmurray commented 4 years ago

Following my comments on #1396, here is a PR for proper MOSEK 9 support. The implementation accomplishes the same thing as #1374 , except it uses the "Optimizer" interface instead of the "Fusion" interface. The Optimizer interface is considered more stable, and is faster than the Fusion interface.

All solver related tests are passing. If you run the full suite of tests, you'll find that five are failing due to pint complaining about unit conversions, and three are failing due to string-rendering / pretty-printing of signomials.

Here is natural follow-up work, if this PR is to be merged:

acdl-jenkins commented 4 years ago

Can one of the admins verify this patch?

1ozturkbe commented 4 years ago

add to whitelist

1ozturkbe commented 4 years ago

test this please

1ozturkbe commented 4 years ago

test models please

1ozturkbe commented 4 years ago

@rileyjmurray this looks great! I agree with you that there is a lot of duplicated code, and that this addition is going to revamp GPkit install, and make is easier and better! @bqpd and I will take on the fixes for the failed tests. We will follow up with you here!

bqpd commented 4 years ago

whitelist this

bqpd commented 4 years ago

test this please

rileyjmurray commented 4 years ago

I see that the Jenkins build failed, with cause

gpkit/_mosek/mosek_conif.py:100: [E1101(no-member), mskoptimize] Instance of 'conetype' has no 'pexp' member

The issue is that when I checked if the mosek_conif interface should be enabled, I just checked that you can call import mosek. In reality, I should have checked that the version of mosek was >= 9. When you're ready I can make that change.

1ozturkbe commented 4 years ago

@rileyjmurray feel free to make the change. I have both versions of mosek atm, as well as Python 2 and 3, so I can check all configurations of solvers and versions of py. It should be straightforward to figure out the issues.

1ozturkbe commented 4 years ago

@rileyjmurray do you know if I can make edits to your fork directly in the master branch, so I can try to get tests to pass? (I can also help with the tedious lint stuff...)

whoburg commented 4 years ago

Thanks @rileyjmurray !!

rileyjmurray commented 4 years ago

@1ozturkbe I made changes to fix the import error, and most of the pylint stuff. You also now have push access to my fork.

rileyjmurray commented 4 years ago

@bqpd I borrowed some ideas from your mosek9 branch to simplify this implementation; see my last commit.

bqpd commented 4 years ago

Ah lol I just pushed up another branch at https://github.com/convexengineering/gpkit/pull/1454 ! will incorporate your latest commit. (you should have push access to that branch)

rileyjmurray commented 4 years ago

@bqpd I'll just give you push access to the master branch of my fork. That way you can edit this PR directly.

https://github.com/rileyjmurray/gpkit/invitations

bqpd commented 4 years ago

The new formulation is noticeably faster, fantastic.

bqpd commented 4 years ago

I'm curious to also incorporate warm starts / efficient reformulations for sweeps / SP solves

bqpd commented 4 years ago

Having trouble pushing to your master branch, so I merged the new formulation into https://github.com/convexengineering/gpkit/pull/1454

bqpd commented 4 years ago

@rileyjmurray re: MIGP, formulating the variables so they'll be integers once exponentiated seems tricky, since you can only specify that exp(x) <= y, y integer, right?

rileyjmurray commented 4 years ago

@bqpd , MIGP is actually not that bad. There is definitely a way to do it, but specifying the problem to mosek's optimizer interface becomes a little more complicated. Also, it's hard to predict how well mosek will cope with the overall formulation. We can discuss more after this PR (or #1454 ) is merged.

rileyjmurray commented 4 years ago

I'm closing this, since #1454 is now the active version of the PR.