-
Notifications
You must be signed in to change notification settings - Fork 140
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
Interface for Gurobi through the gurobipy API #45
Conversation
Hi @RichardOberdieck, thanks for your contribution! We'll have a look and get back to you soon. Small first comment, |
Ok, sure. Shall I keep any of it or shall I just delete |
@RichardOberdieck the whole Since the interface of new |
Removed as per Stefan's comment
When I ran lint on my machine everything worked well. Are these failing checks a consequence of the merge conflicts? If so, shall I have a look at those? |
Gurobipy requires an installation from a private Pypi site. Until it goes to the public Pypi it cannot be included in setup.py because it will fail any install that doesn't have guropbipy installed. I added an install of gurobipy to the CI in order to test it. |
That's fair. Gurobi is currently in the process of getting to the public PyPI server, but there are some bureaucratic steps in the way. I'll let you know here when
The key here is actually that it is set during the environment creation, rather than on the model level. Will you fix this in the PR or shall I? |
What is the license of |
When installing through pip, a non-production license limited to 2000 variables and linear constraints is issued. Otherwise, the user has to get a Gurobi license on www.gurobi.com. For academics, this is free. For commercial users, they will need to purchase a license from Gurobi. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Could you write a release note following the guide? |
@t-imamichi, @manoelmarques : let me know if I can contribute to the release note. |
Thanks. Could you check out the following guide and write a reno? |
@RichardOberdieck If you cannot write a reno, we can write one on be half of you and merge this PR. |
@t-imamichi : I'd appreciate that. Due to the refactor of |
Thanks. I will do that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Summary
Currently, qiskit only works with docplex as the LP/QP/MIP optimization engine. This pull request adds the ability to use Gurobi as well.
Details and comments
Gurobi is a commercial optimization solver (like CPLEX). However, at the moment it is only possible to use cplex if one uses qiskit. As I see that qiskit will grow in the future, I believe that it adds value to enable the use of other optimization engines as well, specifically Gurobi in this case.
I was quite careful to preserve the style of the original codebase, as I mostly copied the
docplex
methods and replaced the API calls with the correspondinggurobipy
calls. All the tests pass, and I added some where I found them to be relevant.The only question I was unable to answer comes in
test_quadratic_program.py
in line 830-831 of my PR. Specifically, the LP formats between Gurobi and cplex differ, and therefore that test does not carry over from the corresponding test fordocplex
. Therefore I decided to comment it out as fixing it would require adding additional logic to the__repr__
method ofQuadraticProgram
. I believe this is not very clean and therefore commented it out. If you have a better idea, then please do share.Also, if you have any other comments/questions/remarks, please share.