From 4b292647e12e262dbefcdd66c5fb956e1312c8f2 Mon Sep 17 00:00:00 2001 From: KristianJensen Date: Wed, 9 Dec 2015 14:28:54 +0100 Subject: [PATCH 1/8] Classes inherit parent's docstring --- optlang/cplex_interface.py | 8 ++++++-- optlang/glpk_interface.py | 12 ++++++++---- optlang/util.py | 15 +++++++++++++++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/optlang/cplex_interface.py b/optlang/cplex_interface.py index 74925273..67895376 100644 --- a/optlang/cplex_interface.py +++ b/optlang/cplex_interface.py @@ -35,6 +35,7 @@ from sympy.core.singleton import S import cplex from optlang import interface +from optlang.util import inheritdocstring Zero = S.Zero One = S.One @@ -146,8 +147,8 @@ def _constraint_lb_and_ub_to_cplex_sense_rhs_and_range_value(lb, ub): return sense, rhs, range_value +@six.add_metaclass(inheritdocstring) class Variable(interface.Variable): - """CPLEX variable interface.""" def __init__(self, name, *args, **kwargs): super(Variable, self).__init__(name, **kwargs) @@ -195,8 +196,8 @@ def dual(self): return None +@six.add_metaclass(inheritdocstring) class Constraint(interface.Constraint): - """CPLEX solver interface""" _INDICATOR_CONSTRAINT_SUPPORT = True @@ -308,6 +309,7 @@ def __iadd__(self, other): return self +@six.add_metaclass(inheritdocstring) class Objective(interface.Objective): def __init__(self, *args, **kwargs): super(Objective, self).__init__(*args, **kwargs) @@ -327,6 +329,7 @@ def __setattr__(self, name, value): super(Objective, self).__setattr__(name, value) +@six.add_metaclass(inheritdocstring) class Configuration(interface.MathematicalProgrammingConfiguration): def __init__(self, lp_method='primal', tolerance=1e-9, presolve=False, verbosity=0, timeout=None, @@ -500,6 +503,7 @@ def qp_method(self, value): self._qp_method = value +@six.add_metaclass(inheritdocstring) class Model(interface.Model): def __init__(self, problem=None, *args, **kwargs): diff --git a/optlang/glpk_interface.py b/optlang/glpk_interface.py index a7c96c95..f1a8b7ce 100644 --- a/optlang/glpk_interface.py +++ b/optlang/glpk_interface.py @@ -28,6 +28,8 @@ from sympy.core.add import _unevaluated_Add from sympy.core.mul import _unevaluated_Mul +from optlang.util import inheritdocstring + import logging log = logging.getLogger(__name__) @@ -67,8 +69,8 @@ ) +@six.add_metaclass(inheritdocstring) class Variable(interface.Variable): - """...""" def __init__(self, name, index=None, *args, **kwargs): super(Variable, self).__init__(name, **kwargs) @@ -137,8 +139,8 @@ def __setattr__(self, name, value): glp_set_col_name(self.problem.problem, glp_find_col(self.problem.problem, old_name), str(value)) +@six.add_metaclass(inheritdocstring) class Constraint(interface.Constraint): - """GLPK solver interface""" _INDICATOR_CONSTRAINT_SUPPORT = False @@ -284,6 +286,7 @@ def __idiv__(self, other): return self +@six.add_metaclass(inheritdocstring) class Objective(interface.Objective): def __init__(self, *args, **kwargs): super(Objective, self).__init__(*args, **kwargs) @@ -349,8 +352,8 @@ def __idiv__(self, other): return self +@six.add_metaclass(inheritdocstring) class Configuration(interface.MathematicalProgrammingConfiguration): - """docstring for Configuration""" def __init__(self, presolve=False, verbosity=0, timeout=None, *args, **kwargs): super(Configuration, self).__init__(*args, **kwargs) @@ -433,8 +436,9 @@ def timeout(self, value): self._set_timeout(value) self._timeout = value + +@six.add_metaclass(inheritdocstring) class Model(interface.Model): - """GLPK solver interface""" def __init__(self, problem=None, *args, **kwargs): diff --git a/optlang/util.py b/optlang/util.py index 2563aea0..c1cde054 100644 --- a/optlang/util.py +++ b/optlang/util.py @@ -23,6 +23,7 @@ log = logging.getLogger(__name__) import tempfile +import inspect from subprocess import check_output @@ -134,6 +135,20 @@ def list_available_solvers(): return solvers +def inheritdocstring(name, bases, attrs): + """Use as metaclass to inherit parent class' docstring. + From http://stackoverflow.com/questions/13937500/inherit-a-parent-class-docstring-as-doc-attribute""" + if '__doc__' not in attrs or not attrs["__doc__"]: + # create a temporary 'parent' to (greatly) simplify the MRO search + temp = type('temporaryclass', bases, {}) + for cls in inspect.getmro(temp): + if cls.__doc__ is not None: + attrs['__doc__'] = cls.__doc__ + break + + return type(name, bases, attrs) + + if __name__ == '__main__': from swiglpk import glp_create_prob, glp_read_lp, glp_get_num_rows From 5a8b6e37e4163256b445654c53fd56c23e48d1a1 Mon Sep 17 00:00:00 2001 From: KristianJensen Date: Wed, 9 Dec 2015 15:36:44 +0100 Subject: [PATCH 2/8] Also inherit method docstrings --- optlang/util.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/optlang/util.py b/optlang/util.py index c1cde054..5e50fcfd 100644 --- a/optlang/util.py +++ b/optlang/util.py @@ -136,8 +136,8 @@ def list_available_solvers(): def inheritdocstring(name, bases, attrs): - """Use as metaclass to inherit parent class' docstring. - From http://stackoverflow.com/questions/13937500/inherit-a-parent-class-docstring-as-doc-attribute""" + """Use as metaclass to inherit class and method docstrings from parent. + Adapted from http://stackoverflow.com/questions/13937500/inherit-a-parent-class-docstring-as-doc-attribute""" if '__doc__' not in attrs or not attrs["__doc__"]: # create a temporary 'parent' to (greatly) simplify the MRO search temp = type('temporaryclass', bases, {}) @@ -146,9 +146,24 @@ def inheritdocstring(name, bases, attrs): attrs['__doc__'] = cls.__doc__ break + for attr_name, attr in attrs.items(): + if not attr.__doc__: + for cls in inspect.getmro(temp): + try: + if getattr(cls, attr_name).__doc__ is not None: + attr.__doc__ = getattr(cls, attr_name).__doc__ + break + except AttributeError: + continue + return type(name, bases, attrs) +def method_inheritdocstring(mthd): + """Use as decorator on a method to inherit doc from parent method of same name""" + if not mthd.__doc__: + pass + if __name__ == '__main__': from swiglpk import glp_create_prob, glp_read_lp, glp_get_num_rows From ced32601cee6eb0a67ec5fa320d464afa7da7cb0 Mon Sep 17 00:00:00 2001 From: KristianJensen Date: Thu, 7 Jan 2016 16:32:57 +0100 Subject: [PATCH 3/8] Removed setattr hacks from glpk --- optlang/glpk_interface.py | 58 +++++++++++++++++++-------------------- optlang/interface.py | 45 ++++++++++++++++++++++++++---- 2 files changed, 67 insertions(+), 36 deletions(-) diff --git a/optlang/glpk_interface.py b/optlang/glpk_interface.py index f1a8b7ce..1cfa117c 100644 --- a/optlang/glpk_interface.py +++ b/optlang/glpk_interface.py @@ -128,15 +128,11 @@ def dual(self): else: return None - def __setattr__(self, name, value): - try: - old_name = self.name # TODO: This is a hack - except AttributeError: - pass - super(Variable, self).__setattr__(name, value) - if getattr(self, 'problem', None): - if name == 'name': - glp_set_col_name(self.problem.problem, glp_find_col(self.problem.problem, old_name), str(value)) + @interface.Variable.name.setter + def name(self, value): + if getattr(self, 'problem', None) is not None: + glp_set_col_name(self.problem.problem, glp_find_col(self.problem.problem, self.name), str(value)) + self._name = value @six.add_metaclass(inheritdocstring) @@ -185,6 +181,24 @@ def _set_coefficients_low_level(self, variables_coefficients_dict): else: raise Exception('_set_coefficients_low_level works only if a constraint is associated with a solver instance.') + @interface.Constraint.lb.setter + def lb(self, value): + self._lb = value + if self.problem is not None: + self.problem._glpk_set_row_bounds(self) + + @interface.Constraint.ub.setter + def ub(self, value): + self._ub = value + if self.problem is not None: + self.problem._glpk_set_row_bounds(self) + + @interface.OptimizationExpression.name.setter + def name(self, value): + if self.problem is not None: + glp_set_row_name(self.problem.problem, glp_find_row(self.problem.problem, self.name), str(value)) + self._name = value + @property def problem(self): return self._problem @@ -237,18 +251,6 @@ def problem(self, value): else: self._problem = value - def __setattr__(self, name, value): - try: - old_name = self.name # TODO: This is a hack - except AttributeError: - pass - super(Constraint, self).__setattr__(name, value) - if getattr(self, 'problem', None): - if name == 'name': - glp_set_row_name(self.problem.problem, glp_find_row(self.problem.problem, old_name), str(value)) - elif name == 'lb' or name == 'ub': - self.problem._glpk_set_row_bounds(self) - def __iadd__(self, other): # if self.problem is not None: # self.problem._add_to_constraint(self.index, other) @@ -313,15 +315,11 @@ def value(self): else: return glp_get_obj_val(self.problem.problem) - def __setattr__(self, name, value): - - if getattr(self, 'problem', None): - if name == 'direction': - glp_set_obj_dir(self.problem.problem, - {'min': GLP_MIN, 'max': GLP_MAX}[value]) - super(Objective, self).__setattr__(name, value) - else: - super(Objective, self).__setattr__(name, value) + @interface.Objective.direction.setter + def direction(self, value): + if getattr(self, 'problem', None) is not None: + glp_set_obj_dir(self.problem.problem, {'min': GLP_MIN, 'max': GLP_MAX}[value]) + super(Objective, self).__setattr__("objective", value) def __iadd__(self, other): self.problem = None diff --git a/optlang/interface.py b/optlang/interface.py index 3b84aac9..d69bc92c 100644 --- a/optlang/interface.py +++ b/optlang/interface.py @@ -135,6 +135,7 @@ def __init__(self, name, lb=None, ub=None, type="continuous", problem=None, *arg raise ValueError( 'Variable names cannot contain whitespace characters. "%s" contains whitespace character "%s".' % ( name, char)) + self._name = name sympy.Symbol.__init__(name, *args, **kwargs) #TODO: change this back to use super self._lb = lb self._ub = ub @@ -147,6 +148,14 @@ def __init__(self, name, lb=None, ub=None, type="continuous", problem=None, *arg self._type = type self.problem = problem + @property + def name(self): + return self._name + + @name.setter + def name(self, value): + self._name = value + @property def lb(self): return self._lb @@ -278,15 +287,23 @@ def __init__(self, expression, name=None, problem=None, sloppy=False, *args, **k name = str(name) super(OptimizationExpression, self).__init__(*args, **kwargs) + self._problem = problem if sloppy: self._expression = expression else: self._expression = self._canonicalize(expression) if name is None: - self.name = str(uuid.uuid1()) + self._name = str(uuid.uuid1()) else: - self.name = name - self._problem = problem + self._name = name + + @property + def name(self): + return self._name + + @name.setter + def name(self, value): + self._name = value @property def problem(self): @@ -439,13 +456,29 @@ def clone(cls, constraint, model=None, **kwargs): name=constraint.name, sloppy=True, **kwargs) def __init__(self, expression, lb=None, ub=None, indicator_variable=None, active_when=1, *args, **kwargs): - self.lb = lb - self.ub = ub + self._lb = lb + self._ub = ub + super(Constraint, self).__init__(expression, *args, **kwargs) self.__check_valid_indicator_variable(indicator_variable) self.__check_valid_active_when(active_when) self._indicator_variable = indicator_variable self._active_when = active_when - super(Constraint, self).__init__(expression, *args, **kwargs) + + @property + def lb(self): + return self._lb + + @lb.setter + def lb(self, value): + self._lb = value + + @property + def ub(self): + return self._ub + + @ub.setter + def ub(self, value): + self._ub = value @property def indicator_variable(self): From 3950bbbaf45a849c255d7d7f927755e07c30992e Mon Sep 17 00:00:00 2001 From: KristianJensen Date: Thu, 7 Jan 2016 16:48:26 +0100 Subject: [PATCH 4/8] Refactored setattr hacks in cplex --- optlang/cplex_interface.py | 101 +++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 48 deletions(-) diff --git a/optlang/cplex_interface.py b/optlang/cplex_interface.py index 67895376..5f96a6ea 100644 --- a/optlang/cplex_interface.py +++ b/optlang/cplex_interface.py @@ -255,46 +255,53 @@ def dual(self): else: return None - # TODO: Refactor to use properties - def __setattr__(self, name, value): - try: - old_name = self.name # TODO: This is a hack - except AttributeError: - pass - super(Constraint, self).__setattr__(name, value) - if getattr(self, 'problem', None): + @interface.Constraint.name.setter + def name(self, value): + if getattr(self, 'problem', None) is not None: + if self.indicator_variable is not None: + raise NotImplementedError( + "Unfortunately, the CPLEX python bindings don't support changing an indicator constraint's name" + ) + else: + # TODO: the following needs to deal with quadratic constraints + self.problem.problem.linear_constraints.set_names(self.name, value) + self._name = value - if name == 'name': - if self.indicator_variable is not None: - raise NotImplementedError("Unfortunately, the CPLEX python bindings don't support changing an indicator constraint's name") - else: - # TODO: the following needs to deal with quadratic constraints - self.problem.problem.linear_constraints.set_names(old_name, value) - - elif name == 'lb' or name == 'ub': - if self.indicator_variable is not None: - raise NotImplementedError("Unfortunately, the CPLEX python bindings don't support changing an indicator constraint's bounds") - if name == 'lb': - if value > self.ub: - raise ValueError( - "Lower bound %f is larger than upper bound %f in constraint %s" % - (value, self.ub, self) - ) - sense, rhs, range_value = _constraint_lb_and_ub_to_cplex_sense_rhs_and_range_value(value, self.ub) - elif name == 'ub': - if value < self.lb: - raise ValueError( - "Upper bound %f is less than lower bound %f in constraint %s" % - (value, self.lb, self) - ) - sense, rhs, range_value = _constraint_lb_and_ub_to_cplex_sense_rhs_and_range_value(self.lb, value) - if self.is_Linear: - self.problem.problem.linear_constraints.set_rhs(self.name, rhs) - self.problem.problem.linear_constraints.set_senses(self.name, sense) - self.problem.problem.linear_constraints.set_range_values(self.name, range_value) - - elif name == 'expression': - pass + @interface.Constraint.lb.setter + def lb(self, value): + if getattr(self, 'problem', None) is not None: + if self.indicator_variable is not None: + raise NotImplementedError( + "Unfortunately, the CPLEX python bindings don't support changing an indicator constraint's bounds" + ) + if value > self.ub: + raise ValueError( + "Lower bound %f is larger than upper bound %f in constraint %s" % + (value, self.ub, self) + ) + sense, rhs, range_value = _constraint_lb_and_ub_to_cplex_sense_rhs_and_range_value(value, self.ub) + if self.is_Linear: + self.problem.problem.linear_constraints.set_rhs(self.name, rhs) + self.problem.problem.linear_constraints.set_senses(self.name, sense) + self.problem.problem.linear_constraints.set_range_values(self.name, range_value) + + @interface.Constraint.ub.setter + def ub(self, value): + if getattr(self, 'problem', None) is not None: + if self.indicator_variable is not None: + raise NotImplementedError( + "Unfortunately, the CPLEX python bindings don't support changing an indicator constraint's bounds" + ) + if value < self.lb: + raise ValueError( + "Upper bound %f is less than lower bound %f in constraint %s" % + (value, self.lb, self) + ) + sense, rhs, range_value = _constraint_lb_and_ub_to_cplex_sense_rhs_and_range_value(self.lb, value) + if self.is_Linear: + self.problem.problem.linear_constraints.set_rhs(self.name, rhs) + self.problem.problem.linear_constraints.set_senses(self.name, sense) + self.problem.problem.linear_constraints.set_range_values(self.name, range_value) def __iadd__(self, other): # if self.problem is not None: @@ -318,15 +325,13 @@ def __init__(self, *args, **kwargs): def value(self): return self.problem.problem.solution.get_objective_value() - def __setattr__(self, name, value): - - if getattr(self, 'problem', None): - if name == 'direction': - self.problem.problem.objective.set_sense( - {'min': self.problem.problem.objective.sense.minimize, 'max': self.problem.problem.objective.sense.maximize}[value]) - super(Objective, self).__setattr__(name, value) - else: - super(Objective, self).__setattr__(name, value) + @interface.Objective.direction.setter + def direction(self, value): + if getattr(self, 'problem', None) is not None: + self.problem.problem.objective.set_sense( + {'min': self.problem.problem.objective.sense.minimize, + 'max': self.problem.problem.objective.sense.maximize}[value]) + super(Objective, self).__setattr__("direction", value) @six.add_metaclass(inheritdocstring) From 1e4f06523a11bbdabba401b927c6b74e7c0b90fc Mon Sep 17 00:00:00 2001 From: KristianJensen Date: Fri, 8 Jan 2016 11:26:22 +0100 Subject: [PATCH 5/8] Round constraint primal to bounds Commented out for now since rounding makes tests fail --- optlang/cplex_interface.py | 4 +++- optlang/glpk_interface.py | 4 +++- optlang/interface.py | 12 ++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/optlang/cplex_interface.py b/optlang/cplex_interface.py index 5f96a6ea..87381bcc 100644 --- a/optlang/cplex_interface.py +++ b/optlang/cplex_interface.py @@ -244,7 +244,9 @@ def problem(self, value): @property def primal(self): if self.problem is not None: - return self.problem.problem.solution.get_activity_levels(self.name) + primal_from_solver = self.problem.problem.solution.get_activity_levels(self.name) + #return self._round_primal_to_bounds(primal_from_solver) # Test assertions fail + return primal_from_solver else: return None diff --git a/optlang/glpk_interface.py b/optlang/glpk_interface.py index 1cfa117c..17a9940d 100644 --- a/optlang/glpk_interface.py +++ b/optlang/glpk_interface.py @@ -227,7 +227,9 @@ def index(self): @property def primal(self): if self.problem is not None: - return glp_get_row_prim(self.problem.problem, self.index) + primal_from_solver = glp_get_row_prim(self.problem.problem, self.index) + #return self._round_primal_to_bounds(primal_from_solver) # Test assertions fail + return primal_from_solver else: return None diff --git a/optlang/interface.py b/optlang/interface.py index d69bc92c..13850970 100644 --- a/optlang/interface.py +++ b/optlang/interface.py @@ -541,6 +541,18 @@ def primal(self): def dual(self): return None + def _round_primal_to_bounds(self, primal, tolerance=1e-5): + if (self.lb is None or primal >= self.lb) and (self.ub is None or primal <= self.ub): + return primal + else: + if (primal <= self.lb) and ((self.lb - primal) <= tolerance): + return self.lb + elif (primal >= self.ub) and ((self.ub - primal) >= -tolerance): + return self.ub + else: + raise AssertionError('The primal value %s returned by the solver is out of bounds for variable %s (lb=%s, ub=%s)' % (primal, self.name, self.lb, self.ub)) + + class Objective(OptimizationExpression): """Objective function. From 0d7b5f8eb67c0639fb5311ee9adb4d93e1704c7d Mon Sep 17 00:00:00 2001 From: KristianJensen Date: Thu, 21 Jan 2016 14:45:55 +0100 Subject: [PATCH 6/8] Fixed constraint bound comparison --- optlang/cplex_interface.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optlang/cplex_interface.py b/optlang/cplex_interface.py index 87381bcc..7af0c61f 100644 --- a/optlang/cplex_interface.py +++ b/optlang/cplex_interface.py @@ -276,7 +276,7 @@ def lb(self, value): raise NotImplementedError( "Unfortunately, the CPLEX python bindings don't support changing an indicator constraint's bounds" ) - if value > self.ub: + if self.ub is not None and value > self.ub: raise ValueError( "Lower bound %f is larger than upper bound %f in constraint %s" % (value, self.ub, self) @@ -294,7 +294,7 @@ def ub(self, value): raise NotImplementedError( "Unfortunately, the CPLEX python bindings don't support changing an indicator constraint's bounds" ) - if value < self.lb: + if self.lb is not None and value < self.lb: raise ValueError( "Upper bound %f is less than lower bound %f in constraint %s" % (value, self.lb, self) From ad226740bf67387c9c2859bda2910bc88fa1132d Mon Sep 17 00:00:00 2001 From: KristianJensen Date: Fri, 22 Jan 2016 11:00:22 +0100 Subject: [PATCH 7/8] Fixed error when setting bounds --- optlang/cplex_interface.py | 2 ++ optlang/interface.py | 1 - tests/test_cplex_interface.py | 7 +++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/optlang/cplex_interface.py b/optlang/cplex_interface.py index 7af0c61f..7a05f7b5 100644 --- a/optlang/cplex_interface.py +++ b/optlang/cplex_interface.py @@ -286,6 +286,7 @@ def lb(self, value): self.problem.problem.linear_constraints.set_rhs(self.name, rhs) self.problem.problem.linear_constraints.set_senses(self.name, sense) self.problem.problem.linear_constraints.set_range_values(self.name, range_value) + self._lb = value @interface.Constraint.ub.setter def ub(self, value): @@ -304,6 +305,7 @@ def ub(self, value): self.problem.problem.linear_constraints.set_rhs(self.name, rhs) self.problem.problem.linear_constraints.set_senses(self.name, sense) self.problem.problem.linear_constraints.set_range_values(self.name, range_value) + self._ub = value def __iadd__(self, other): # if self.problem is not None: diff --git a/optlang/interface.py b/optlang/interface.py index 13850970..1b5f637b 100644 --- a/optlang/interface.py +++ b/optlang/interface.py @@ -553,7 +553,6 @@ def _round_primal_to_bounds(self, primal, tolerance=1e-5): raise AssertionError('The primal value %s returned by the solver is out of bounds for variable %s (lb=%s, ub=%s)' % (primal, self.name, self.lb, self.ub)) - class Objective(OptimizationExpression): """Objective function. diff --git a/tests/test_cplex_interface.py b/tests/test_cplex_interface.py index e10f6285..05ed16c8 100644 --- a/tests/test_cplex_interface.py +++ b/tests/test_cplex_interface.py @@ -116,6 +116,13 @@ def test_setting_nonnumerical_bounds_raises(self): model = Model(problem=problem) self.assertRaises(Exception, setattr, model.constraints[0], 'lb', 'Chicken soup') + def test_setting_bounds(self): + value = 42 + self.constraint.ub = value + self.assertEqual(self.constraint.ub, value) + self.constraint.lb = value + self.assertEqual(self.constraint.lb, value) + class SolverTestCase(unittest.TestCase): def setUp(self): From 9b6377e763ccb0b8f6c177d60a8c1fb164f2636f Mon Sep 17 00:00:00 2001 From: KristianJensen Date: Mon, 1 Feb 2016 13:44:29 +0100 Subject: [PATCH 8/8] Fixed python 2 __doc__ issue In Python 2 trying to set a readonly attribute raises TypeError instead of AttributeError --- optlang/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optlang/util.py b/optlang/util.py index 5e50fcfd..3e934e8c 100644 --- a/optlang/util.py +++ b/optlang/util.py @@ -153,7 +153,7 @@ def inheritdocstring(name, bases, attrs): if getattr(cls, attr_name).__doc__ is not None: attr.__doc__ = getattr(cls, attr_name).__doc__ break - except AttributeError: + except (AttributeError, TypeError): continue return type(name, bases, attrs)