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

support for mosek 9 #1452

Closed
wants to merge 21 commits into from
Closed

support for mosek 9 #1452

wants to merge 21 commits into from

Conversation

rileyjmurray
Copy link
Contributor

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:

  • Allow handling for more user-provided MOSEK parameters. To do this well, refer to CVXPY's MOSEK interface, specifically lines 228-241 and 571-602.
  • Allow mixed-integer geometric programming. This is doable, but it will require an unusual formulation. Once you've settled on a formulation, updating this mosek interface shouldn't be that hard. If need be, I can help with that when the time comes.
  • Fix non-solver-related tests that are currently failing.
  • Drop the other mosek interfaces. The code duplication is pretty bad right now. Also, if you drop mosek_cli and the C-based mosek interface, installing GPKit will become much easier. You wouldn't need a "build" phase at all!

@acdl-jenkins
Copy link

Can one of the admins verify this patch?

@1ozturkbe
Copy link
Contributor

add to whitelist

@1ozturkbe
Copy link
Contributor

test this please

@1ozturkbe
Copy link
Contributor

test models please

@1ozturkbe
Copy link
Contributor

1ozturkbe commented Jan 21, 2020

@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!

@1ozturkbe 1ozturkbe requested a review from bqpd January 21, 2020 15:33
@bqpd
Copy link
Contributor

bqpd commented Jan 21, 2020

whitelist this

@bqpd
Copy link
Contributor

bqpd commented Jan 21, 2020

test this please

@rileyjmurray
Copy link
Contributor Author

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
Copy link
Contributor

@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
Copy link
Contributor

1ozturkbe commented Jan 21, 2020

@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
Copy link
Collaborator

whoburg commented Jan 21, 2020

Thanks @rileyjmurray !!

@rileyjmurray
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

@bqpd
Copy link
Contributor

bqpd commented Jan 22, 2020

Ah lol I just pushed up another branch at #1454 ! will incorporate your latest commit. (you should have push access to that branch)

@rileyjmurray
Copy link
Contributor Author

rileyjmurray commented Jan 22, 2020

@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
Copy link
Contributor

bqpd commented Jan 22, 2020

The new formulation is noticeably faster, fantastic.

@bqpd
Copy link
Contributor

bqpd commented Jan 22, 2020

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

@bqpd
Copy link
Contributor

bqpd commented Jan 22, 2020

Having trouble pushing to your master branch, so I merged the new formulation into #1454

@bqpd
Copy link
Contributor

bqpd commented Jan 22, 2020

@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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

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

Successfully merging this pull request may close these issues.

None yet

5 participants