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

Capture Cplex specific errors and raise a unified optlang.SolverError [WIP] #94

Closed
wants to merge 11 commits into from

Conversation

Midnighter
Copy link
Member

@Midnighter Midnighter commented Apr 5, 2017

When working with different optlang solver interfaces error handling should be agnostic. So far this is implemented for Cplex and Gurobi should come next. As far as I'm aware GLPK raises no errors but maybe surprising behavior should still be handled by raising from optlang. I'm not sure.

So far this pull request also implements test cases for accessing dual values on MIP problems. These should go into a separate branch/PR, I suppose.

Tests cannot pass at the moment since I introduce two failing tests due to the above.

This introduces the unified optlang error that should be reraised anywhere that
solver specific errors can occur. Third party code can then rely on catching the
optlang error only.
Using `future` reliably allows us to reraise in Python 2 & 3.
As a basis AbstractDualValuesMIPTestCase expects dual values access of any shape
or form to raise a SolverError exception for now. This will change in future and
only Cplex raises.
Also implemented specific Cplex test cases InfeasibleMIPTestCase that test an
infeasible MIP and expect certain kinds of errors.
@@ -31,14 +31,17 @@

log = logging.getLogger(__name__)

from future.utils import raise_from
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should stick with the six module instead of using two different compatibility libraries.

Copy link
Member Author

Choose a reason for hiding this comment

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

future is built on top of six and I find it a lot nicer in many cases but sure, I can drop it.

@@ -497,6 +499,98 @@ def test_qp_non_convex(self):
model.optimize()
self.assertAlmostEqual(model.objective.value, 2441.999999971)

class InfeasibleMIPTestCase(unittest.TestCase):
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't this be combined with the other test case?

Copy link
Member Author

Choose a reason for hiding this comment

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

I could drop those cases that are tested by accessing dual values in MIP already, true. However, I want to test here that all Cplex errors are adequately caught and reraised as optlang.SolverErrors. That behavior is different between MIPs and LPs. Maybe I should extend this to LPs and QPs, too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that it's good to test this as well, and I just meant that these tests could simply be added to the other test case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather keep them separate since the intention of the tests is something different.

@codecov-io
Copy link

codecov-io commented Apr 5, 2017

Codecov Report

Merging #94 into devel will decrease coverage by 0.17%.
The diff coverage is 82.6%.

Impacted file tree graph

@@            Coverage Diff            @@
##           devel      #94      +/-   ##
=========================================
- Coverage   88.8%   88.63%   -0.18%     
=========================================
  Files          8        8              
  Lines       2386     2419      +33     
  Branches     542      542              
=========================================
+ Hits        2119     2144      +25     
- Misses       166      174       +8     
  Partials     101      101
Impacted Files Coverage Δ
optlang/exceptions.py 100% <100%> (ø) ⬆️
optlang/cplex_interface.py 88.92% <81.39%> (-0.92%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ec31da...871902a. Read the comment docs.

@Midnighter Midnighter changed the title Capture solver specific errors and raise a unified optlang.SolverError Capture Cplex specific errors and raise a unified optlang.SolverError [WIP] Apr 6, 2017
@Midnighter
Copy link
Member Author

Closing due to #96 which is a polished version of this branch.

@Midnighter Midnighter closed this Apr 6, 2017
@Midnighter Midnighter deleted the feat/single-error branch April 6, 2017 12:04
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.

4 participants