-
Notifications
You must be signed in to change notification settings - Fork 565
add cuopt direct solver #3620
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
base: main
Are you sure you want to change the base?
add cuopt direct solver #3620
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3620 +/- ##
========================================
Coverage 89.45% 89.45%
========================================
Files 905 906 +1
Lines 105467 105735 +268
========================================
+ Hits 94341 94586 +245
- Misses 11126 11149 +23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mrmundt
left a comment
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.
We are working on making cuopt available in our testing infrastructure; can you please add tests to this PR?
|
@Iroy30 - We've been able to make cuopt available on our internal testing machines. Can you please add tests to this PR? |
5dbf9dd to
6f64094
Compare
|
@mrmundt Thanks! We have added tests by enabling testing cuopt with LP and MILP capabilities in tests/solvers.py. Let us know if:
The following is the testing output I get relevant to cuOpt |
| t0 = time.time() | ||
| self.solution = cuopt.linear_programming.solver.Solve(self._solver_model) | ||
| t1 = time.time() |
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.
This is fine, but just so you're aware, we have this lovely little utility called TicTocTimer that you may want to consider using: https://pyomo.readthedocs.io/en/latest/api/pyomo.common.timing.TicTocTimer.html
|
@mrmundt What do you think about including this solver interface in pyomo.contrib.solvers? Would it make sense to pull-in new solver interfaces there, since that's where the new solver API is evolving? |
@whart222 - I am evenly split. Because we are still messing with what the new solver interfaces are going to actually do / how they will handle input and present output, I don't know if we want to put "new" solvers there or just "well-established" ones that we can robustly test / really know what they are supposed to do and return. |
|
@Iroy30 - I forgot to post this last week, but all of the failures are of the variety: |
|
@Iroy30 - Two more things:
|
Co-authored-by: Miranda Mundt <[email protected]>
|
OK. We are getting close. cuopt finally appears to be running on Jenkins. We are seeing 9 failures: |
Fixed by deleting instance level version , now the tests are picked up correctly as expected failures (cuOpt has these known failures) |
|
Great! Looks like the CI passes except codecov/patch , how do we reconcile this, any suggestions? |
The codecov/patch check is failing because |
|
Hi all, does this look good for a final review and merge? |
| import logging | ||
| import re | ||
| import sys | ||
|
|
||
| from pyomo.common.collections import ComponentSet, ComponentMap, Bunch | ||
| from pyomo.common.dependencies import attempt_import | ||
| from pyomo.common.dependencies import numpy as np | ||
| from pyomo.core.base import Suffix, Var, Constraint, SOSConstraint, Objective | ||
| from pyomo.common.errors import ApplicationError | ||
| from pyomo.common.tempfiles import TempfileManager | ||
| from pyomo.common.tee import capture_output | ||
| from pyomo.core.expr.numvalue import is_fixed | ||
| from pyomo.core.expr.numvalue import value | ||
| from pyomo.core.staleflag import StaleFlagManager | ||
| from pyomo.repn import generate_standard_repn | ||
| from pyomo.repn.linear import LinearRepnVisitor | ||
| from pyomo.solvers.plugins.solvers.direct_solver import DirectSolver | ||
| from pyomo.solvers.plugins.solvers.direct_or_persistent_solver import ( | ||
| DirectOrPersistentSolver, | ||
| ) | ||
| from pyomo.common.enums import minimize, maximize | ||
| from pyomo.opt.results.results_ import SolverResults | ||
| from pyomo.opt.results.solution import Solution, SolutionStatus | ||
| from pyomo.opt.results.solver import TerminationCondition, SolverStatus | ||
| from pyomo.opt.base import SolverFactory | ||
| import time |
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.
There are quite a few unused imports here (flagged by my IDE).
| import logging | |
| import re | |
| import sys | |
| from pyomo.common.collections import ComponentSet, ComponentMap, Bunch | |
| from pyomo.common.dependencies import attempt_import | |
| from pyomo.common.dependencies import numpy as np | |
| from pyomo.core.base import Suffix, Var, Constraint, SOSConstraint, Objective | |
| from pyomo.common.errors import ApplicationError | |
| from pyomo.common.tempfiles import TempfileManager | |
| from pyomo.common.tee import capture_output | |
| from pyomo.core.expr.numvalue import is_fixed | |
| from pyomo.core.expr.numvalue import value | |
| from pyomo.core.staleflag import StaleFlagManager | |
| from pyomo.repn import generate_standard_repn | |
| from pyomo.repn.linear import LinearRepnVisitor | |
| from pyomo.solvers.plugins.solvers.direct_solver import DirectSolver | |
| from pyomo.solvers.plugins.solvers.direct_or_persistent_solver import ( | |
| DirectOrPersistentSolver, | |
| ) | |
| from pyomo.common.enums import minimize, maximize | |
| from pyomo.opt.results.results_ import SolverResults | |
| from pyomo.opt.results.solution import Solution, SolutionStatus | |
| from pyomo.opt.results.solver import TerminationCondition, SolverStatus | |
| from pyomo.opt.base import SolverFactory | |
| import time | |
| import logging | |
| import re | |
| import time | |
| from pyomo.common.collections import ComponentSet, ComponentMap, Bunch | |
| from pyomo.common.dependencies import attempt_import | |
| from pyomo.common.dependencies import numpy as np | |
| from pyomo.core.base import Suffix, Var, Constraint, Objective | |
| from pyomo.core.expr.numvalue import is_fixed | |
| from pyomo.core.staleflag import StaleFlagManager | |
| from pyomo.repn import generate_standard_repn | |
| from pyomo.solvers.plugins.solvers.direct_solver import DirectSolver | |
| from pyomo.solvers.plugins.solvers.direct_or_persistent_solver import ( | |
| DirectOrPersistentSolver, | |
| ) | |
| from pyomo.common.enums import minimize, maximize | |
| from pyomo.opt.results.results_ import SolverResults | |
| from pyomo.opt.results.solution import Solution, SolutionStatus | |
| from pyomo.opt.results.solver import TerminationCondition, SolverStatus | |
| from pyomo.opt.base import SolverFactory |
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.
Resolved
| class CUOPTDirect(DirectSolver): | ||
| def __init__(self, **kwds): | ||
| kwds["type"] = "cuoptdirect" | ||
| super(CUOPTDirect, self).__init__(**kwds) |
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.
This is old-style Python. Suggest switching to new-style:
| super(CUOPTDirect, self).__init__(**kwds) | |
| super().__init__(**kwds) |
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.
Updated
| from pyomo.opt.base import SolverFactory | ||
| import time | ||
|
|
||
| logger = logging.getLogger(__name__) |
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.
This is never actually used anywhere. We love our loggers, but why bother declaring it if it's not used? Maybe you can change some of the error messages into log messages and/or add some helpful logging in between steps.
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.
Utilized the logger to log intermediate metadata
- Remove unused imports - Use new-style super() - Add logging (debug for solve, warnings for MIP limitations)
Address requested changes
Fixes #3626
Summary/Motivation:
Add cuOpt math optimization (includes LP and MILP) solver backend to Pyomo so users can solve pyomo models with cuOpt
Changes proposed in this PR:
Legal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution: