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

Gurobipy Environment Handling #588

Merged
merged 14 commits into from
Sep 24, 2023
Merged

Conversation

torressa
Copy link
Contributor

@torressa torressa commented Oct 3, 2022

  • Fix Gurobi: If status not optimal, solution cannot be retrieved #586.
  • For Gurobi environment handling #571
    • Fix issue with pulp.list_solvers. Cheers @simonbowly!
    • Added three new parameters to GUROBI that allow users more fine control over the environments used.
      • env: gp.Env users environment to use.
      • envOptions: dict, options to give to the environment if manageEnv is True.
      • manageEnv: bool: whether environments are handled internally or not default is false (as it was before).
    • Added a new function to free the environments and models properly close. If users are using their own environments or using manageEnv=True, they should always call this function afterwards.

Sample use:

# Pass environment as a parameter
with gp.Env(params=env_options) as env:
  prob = generate_lp()
  solver = GUROBI(msg=True, env=env, **solver_options)
  prob.solve(solver)
  # Dispose model (required to correctly free env)
  solver.close()

Or,

# Use manageEnv=True and pulp will take care
solver = GUROBI(msg=True, manageEnv=True, envOptions=env_options, **solver_options)
prob = generate_lp()
prob.solve(solver)
# Dispose env + model
solver.close()

@CLAassistant
Copy link

CLAassistant commented Oct 3, 2022

CLA assistant check
All committers have signed the CLA.

@EwoutH
Copy link
Contributor

EwoutH commented Dec 11, 2022

This looks quite cool! What's needed to move this further? A review from @pchtsp?

@torressa torressa changed the title Gurobipy updates Gurobipy Environment Handling Jan 11, 2023
pulp/apis/gurobi_api.py Outdated Show resolved Hide resolved
@Tomkourou
Copy link

Commenting to move this along. Problems with the gurobipy single-use licenses are still unresolved.

@torressa
Copy link
Contributor Author

torressa commented May 3, 2023

@simon-b I've just added some usage explanations to the doc-string, should be clearer now.
(tested building the docs on my machine and was OK).
Also rebased master, hope this is merged soon!

@torressa torressa requested a review from simon-b July 20, 2023 13:58
Copy link

@simon-b simon-b left a comment

Choose a reason for hiding this comment

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

Looks great to me! Test on my side with various options for managing the gurobi env and all seem to work properly.

Suggest a review from @pchtsp and hopefully can get this closed soon !

pulp/tests/test_gurobipy_env.py Outdated Show resolved Hide resolved
prob, self.solver, [const.LpStatusOptimal], {x: 4, y: -1, z: 6, w: 0}
)
if self.solver.name not in [
"GUROBI_CMD", # end is a key-word for LP files
Copy link

Choose a reason for hiding this comment

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

Not clear to me why this was changed, when otherwise the PR doesn't seem to make any changes to GUROBI_CMD 🤔

Copy link
Contributor Author

@torressa torressa Aug 1, 2023

Choose a reason for hiding this comment

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

I think this must've not been tested using GUROBI_CMD. Variable z is called End which is a keyword for LP files hence results in the LP file being solved being pretty different

Minimize
  9
Subject To
Bounds
End

(I double-checked versions of Gurobi back to 8.0.0 with the same result)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unless this test is supposed to fail, as the name suggests.

- Add decorator to skip `test_measuring_solving_time` as it exceeds the
  pip-license size limitations.
- Add `test_gurobipy_env` to pulptest which will be skipped if
  `gurobipy` is not installed.
@torressa
Copy link
Contributor Author

torressa commented Aug 4, 2023

The tests should now run using the Gurobi license included when installing gurobipy via pip install (which I've included in the workflow). There is one test where the size limitation is reached, for which I've added a decorator. This test will run fine if a full license is picked up.

Unfortunately, the pip license will not be picked up by gurobi_cl or the GUROBI_CMD interface.

Therefore the best thing would be to generate and use a Gurobi WLS license that can be added to the workflow via a secret.

For now, we can test this by re-running the actions but a maintainer needs to trigger that.

@stumitchell
Copy link
Contributor

@torressa we are getting a test fail now,
Also can we remove the restricted licence warning from the test output?

@torressa
Copy link
Contributor Author

torressa commented Aug 7, 2023

Hi @stumitchell! If you mean the tests on the CI, I am not sure why, the errors are:

 File "/opt/hostedtoolcache/Python/3.9.17/x64/lib/python3.9/site-packages/pulp/apis/highs_api.py", line 327, in buildSolverModel
    lp.solverModel.addCol(
ModuleNotFoundError: No module named 'numpy'

This is weird cause it passes on macos and also cause highspy doesn't use numpy.
It seems that it is broken also on all branches

I tried removing the warning from the output (I had forgotten quite a few places).

@stumitchell
Copy link
Contributor

Right, I see the issue I have contacted @aphi about it

We probably want to get master fixed before we merge anything new

@pchtsp
Copy link
Collaborator

pchtsp commented Aug 8, 2023

Thanks @stumitchell

I was waiting (too) patiently for the numpy issue to resolve in highspy. I did forget to create an issue there : ) so thanks for doing so.

I've currently taken out the installation of the highspy python package from the github CI on the master branch. This should solve this issue.

@torressa can you re-merge master please? Sorry for the lack of response, these months have been very busy.

F.

@aphi
Copy link
Contributor

aphi commented Aug 8, 2023

Looks like we both investigated this at the same time 😬.

I have a PR which keeps the testcase for highspy but just includes numpy, which also resolves the issue.

#677

@torressa
Copy link
Contributor Author

torressa commented Aug 8, 2023

No worries @pchtsp! Merged.
I just opened an issue in the HiGHS repo so that they are aware: ERGO-Code/HiGHS#1378

@pchtsp pchtsp merged commit 42f91ab into coin-or:master Sep 24, 2023
@torressa torressa deleted the gurobipy-environments branch September 27, 2023 09:41
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.

Gurobi: If status not optimal, solution cannot be retrieved
9 participants