diff --git a/CHANGELOG.md b/CHANGELOG.md index fb4b3a96f..bb4d111f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Removed `Py_INCREF`/`Py_DECREF` on `Model` in `catchEvent`/`dropEvent` that caused memory leak for imbalanced usage - Used `getIndex()` instead of `ptr()` for sorting nonlinear expression terms to avoid nondeterministic behavior ### Changed +- Return NotImplemented for `Expr` and `GenExpr` operators, if they can't handle input types in the calculation - Speed up `constant * Expr` via C-level API ### Removed - Removed outdated warning about Make build system incompatibility diff --git a/src/pyscipopt/expr.pxi b/src/pyscipopt/expr.pxi index 59c7c7e4d..3d52d49a0 100644 --- a/src/pyscipopt/expr.pxi +++ b/src/pyscipopt/expr.pxi @@ -43,63 +43,25 @@ # gets called (I guess) and so a copy is returned. # Modifying the expression directly would be a bug, given that the expression might be re-used by the user. import math -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Union + +import numpy as np from cpython.dict cimport PyDict_Next, PyDict_GetItem -from cpython.object cimport Py_TYPE +from cpython.number cimport PyNumber_Check +from cpython.object cimport Py_LE, Py_EQ, Py_GE, Py_TYPE from cpython.ref cimport PyObject from cpython.tuple cimport PyTuple_GET_ITEM -from pyscipopt.scip cimport Variable, Solution -import numpy as np +from numpy cimport PyArray_Check + +from pyscipopt.scip cimport Variable, Solution if TYPE_CHECKING: double = float -def _is_number(e): - try: - f = float(e) - return True - except ValueError: # for malformed strings - return False - except TypeError: # for other types (Variable, Expr) - return False - - -def _expr_richcmp(self, other, op): - if op == 1: # <= - if isinstance(other, Expr) or isinstance(other, GenExpr): - return (self - other) <= 0.0 - elif _is_number(other): - return ExprCons(self, rhs=float(other)) - elif isinstance(other, np.ndarray): - return _expr_richcmp(other, self, 5) - else: - raise TypeError(f"Unsupported type {type(other)}") - elif op == 5: # >= - if isinstance(other, Expr) or isinstance(other, GenExpr): - return (self - other) >= 0.0 - elif _is_number(other): - return ExprCons(self, lhs=float(other)) - elif isinstance(other, np.ndarray): - return _expr_richcmp(other, self, 1) - else: - raise TypeError(f"Unsupported type {type(other)}") - elif op == 2: # == - if isinstance(other, Expr) or isinstance(other, GenExpr): - return (self - other) == 0.0 - elif _is_number(other): - return ExprCons(self, lhs=float(other), rhs=float(other)) - elif isinstance(other, np.ndarray): - return _expr_richcmp(other, self, 2) - else: - raise TypeError(f"Unsupported type {type(other)}") - else: - raise NotImplementedError("Can only support constraints with '<=', '>=', or '=='.") - - cdef class Term: '''This is a monomial term''' @@ -181,8 +143,11 @@ cdef class Term: CONST = Term() # helper function -def buildGenExprObj(expr): +def buildGenExprObj(expr: Union[int, float, np.number, Expr, GenExpr]) -> GenExpr: """helper function to generate an object of type GenExpr""" + if not _is_genexpr_compatible(expr): + raise TypeError(f"unsupported type {type(expr).__name__!s}") + if _is_number(expr): return Constant(expr) @@ -205,15 +170,7 @@ def buildGenExprObj(expr): sumexpr += coef * prodexpr return sumexpr - elif isinstance(expr, np.ndarray): - GenExprs = np.empty(expr.shape, dtype=object) - for idx in np.ndindex(expr.shape): - GenExprs[idx] = buildGenExprObj(expr[idx]) - return GenExprs.view(MatrixExpr) - - else: - assert isinstance(expr, GenExpr) - return expr + return expr ##@details Polynomial expressions of variables with operator overloading. \n #See also the @ref ExprDetails "description" in the expr.pxi. @@ -240,6 +197,9 @@ cdef class Expr: return abs(buildGenExprObj(self)) def __add__(self, other): + if not _is_expr_compatible(other): + return NotImplemented + left = self right = other terms = left.terms.copy() @@ -251,35 +211,23 @@ cdef class Expr: elif _is_number(right): c = float(right) terms[CONST] = terms.get(CONST, 0.0) + c - elif isinstance(right, GenExpr): - return buildGenExprObj(left) + right - elif isinstance(right, np.ndarray): - return right + left - else: - raise TypeError(f"Unsupported type {type(right)}") - return Expr(terms) def __iadd__(self, other): + if not _is_expr_compatible(other): + return NotImplemented + if isinstance(other, Expr): for v,c in other.terms.items(): self.terms[v] = self.terms.get(v, 0.0) + c elif _is_number(other): c = float(other) self.terms[CONST] = self.terms.get(CONST, 0.0) + c - elif isinstance(other, GenExpr): - # is no longer in place, might affect performance? - # can't do `self = buildGenExprObj(self) + other` since I get - # TypeError: Cannot convert pyscipopt.scip.SumExpr to pyscipopt.scip.Expr - return buildGenExprObj(self) + other - else: - raise TypeError(f"Unsupported type {type(other)}") - return self def __mul__(self, other): - if isinstance(other, np.ndarray): - return other * self + if not _is_expr_compatible(other): + return NotImplemented cdef dict res = {} cdef Py_ssize_t pos1 = 0, pos2 = 0 @@ -292,10 +240,9 @@ cdef class Expr: cdef double coef if _is_number(other): - coef = float(other) + coef = other while PyDict_Next(self.terms, &pos1, &k1_ptr, &v1_ptr): res[k1_ptr] = (v1_ptr) * coef - return Expr(res) elif isinstance(other, Expr): while PyDict_Next(self.terms, &pos1, &k1_ptr, &v1_ptr): @@ -307,22 +254,20 @@ cdef class Expr: res[child] = (old_v_ptr) + coef else: res[child] = coef - return Expr(res) + return Expr(res) - elif isinstance(other, GenExpr): - return buildGenExprObj(self) * other - else: - raise NotImplementedError + def __truediv__(self, other): + if not _is_expr_compatible(other): + return NotImplemented - def __truediv__(self,other): if _is_number(other): - f = 1.0/float(other) - return f * self - selfexpr = buildGenExprObj(self) - return selfexpr.__truediv__(other) + return 1.0 / other * self + return buildGenExprObj(self) / other def __rtruediv__(self, other): ''' other / self ''' + if not _is_expr_compatible(other): + return NotImplemented return buildGenExprObj(other) / self def __pow__(self, other, modulo): @@ -341,13 +286,11 @@ cdef class Expr: Implements base**x as scip.exp(x * scip.log(base)). Note: base must be positive. """ - if _is_number(other): - base = float(other) - if base <= 0.0: - raise ValueError("Base of a**x must be positive, as expression is reformulated to scip.exp(x * scip.log(a)); got %g" % base) - return exp(self * log(base)) - else: + if not _is_number(other): raise TypeError(f"Unsupported base type {type(other)} for exponentiation.") + if other <= 0.0: + raise ValueError("Base of a**x must be positive, as expression is reformulated to scip.exp(x * scip.log(a)); got %g" % other) + return exp(self * log(float(other))) def __neg__(self): return Expr({v:-c for v,c in self.terms.items()}) @@ -364,7 +307,7 @@ cdef class Expr: def __rsub__(self, other): return -1.0 * self + other - def __richcmp__(self, other, op): + def __richcmp__(self, other, int op): '''turn it into a constraint''' return _expr_richcmp(self, other, op) @@ -429,14 +372,14 @@ cdef class ExprCons: def __richcmp__(self, other, op): '''turn it into a constraint''' + if not _is_number(other): + raise TypeError('Ranged ExprCons is not well defined!') + if op == 1: # <= if not self._rhs is None: raise TypeError('ExprCons already has upper bound') assert not self._lhs is None - if not _is_number(other): - raise TypeError('Ranged ExprCons is not well defined!') - return ExprCons(self.expr, lhs=self._lhs, rhs=float(other)) elif op == 5: # >= if not self._lhs is None: @@ -444,9 +387,6 @@ cdef class ExprCons: assert self._lhs is None assert not self._rhs is None - if not _is_number(other): - raise TypeError('Ranged ExprCons is not well defined!') - return ExprCons(self.expr, lhs=float(other), rhs=self._rhs) else: raise NotImplementedError("Ranged ExprCons can only support with '<=' or '>='.") @@ -516,8 +456,8 @@ cdef class GenExpr: return UnaryExpr(Operator.fabs, self) def __add__(self, other): - if isinstance(other, np.ndarray): - return other + self + if not _is_genexpr_compatible(other): + return NotImplemented left = buildGenExprObj(self) right = buildGenExprObj(other) @@ -574,8 +514,8 @@ cdef class GenExpr: # return self def __mul__(self, other): - if isinstance(other, np.ndarray): - return other * self + if not _is_genexpr_compatible(other): + return NotImplemented left = buildGenExprObj(self) right = buildGenExprObj(other) @@ -640,16 +580,17 @@ cdef class GenExpr: Implements base**x as scip.exp(x * scip.log(base)). Note: base must be positive. """ - if _is_number(other): - base = float(other) - if base <= 0.0: - raise ValueError("Base of a**x must be positive, as expression is reformulated to scip.exp(x * scip.log(a)); got %g" % base) - return exp(self * log(base)) - else: + if not _is_number(other): raise TypeError(f"Unsupported base type {type(other)} for exponentiation.") + if other <= 0.0: + raise ValueError("Base of a**x must be positive, as expression is reformulated to scip.exp(x * scip.log(a)); got %g" % other) + return exp(self * log(float(other))) #TODO: ipow, idiv, etc def __truediv__(self,other): + if not _is_genexpr_compatible(other): + return NotImplemented + divisor = buildGenExprObj(other) # we can't divide by 0 if isinstance(divisor, GenExpr) and divisor.getOp() == Operator.const and divisor.number == 0.0: @@ -658,8 +599,9 @@ cdef class GenExpr: def __rtruediv__(self, other): ''' other / self ''' - otherexpr = buildGenExprObj(other) - return otherexpr.__truediv__(self) + if not _is_genexpr_compatible(other): + return NotImplemented + return buildGenExprObj(other) / self def __neg__(self): return -1.0 * self @@ -676,7 +618,7 @@ cdef class GenExpr: def __rsub__(self, other): return -1.0 * self + other - def __richcmp__(self, other, op): + def __richcmp__(self, other, int op): '''turn it into a constraint''' return _expr_richcmp(self, other, op) @@ -907,3 +849,37 @@ def expr_to_array(expr, nodes): else: # var nodes.append( tuple( [op, expr.children] ) ) return len(nodes) - 1 + + +cdef inline bint _is_number(object o): + return not PyArray_Check(o) and PyNumber_Check(o) + +cdef inline bint _is_expr_compatible(object o): + return _is_number(o) or isinstance(o, Expr) + +cdef inline bint _is_genexpr_compatible(object o): + return _is_expr_compatible(o) or isinstance(o, GenExpr) + +cdef object _expr_richcmp( + self, + other: Union[int, float, np.number, Expr, GenExpr], + int op, +): + if isinstance(other, np.ndarray): + return NotImplemented + if not _is_genexpr_compatible(other): + raise TypeError(f"unsupported type {type(other).__name__!s}") + + if op == Py_LE: + if _is_number(other): + return ExprCons(self, rhs=other) + return ExprCons(self - other, rhs=0.0) + elif op == Py_GE: + if _is_number(other): + return ExprCons(self, lhs=other) + return ExprCons(self - other, lhs=0.0) + elif op == Py_EQ: + if _is_number(other): + return ExprCons(self, lhs=other, rhs=other) + return ExprCons(self - other, lhs=0.0, rhs=0.0) + raise NotImplementedError("can only support with '<=', '>=', or '=='") diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index 871638ebf..1e2c0d4cf 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -3998,7 +3998,6 @@ cdef class Model: # turn the constant value into an Expr instance for further processing if not isinstance(expr, Expr): - assert(_is_number(expr)), "given coefficients are neither Expr or number but %s" % expr.__class__.__name__ expr = Expr() + expr if expr.degree() > 1: @@ -12750,14 +12749,14 @@ def readStatistics(filename): if stat_name == "Gap": relevant_value = relevant_value[:-1] # removing % - if _is_number(relevant_value): + try: result[stat_name] = float(relevant_value) + except: + result[stat_name] = relevant_value + else: if stat_name == "Solutions found" and result[stat_name] == 0: break - else: # it's a string - result[stat_name] = relevant_value - # changing keys to pythonic variable names treated_keys = {"status": "status", "Total Time": "total_time", "solving":"solving_time", "presolving":"presolving_time", "reading":"reading_time", "copying":"copying_time", "Problem name": "problem_name", "Presolved Problem name": "presolved_problem_name", "Variables":"_variables", diff --git a/src/pyscipopt/scip.pyi b/src/pyscipopt/scip.pyi index 92cc58540..da97994bd 100644 --- a/src/pyscipopt/scip.pyi +++ b/src/pyscipopt/scip.pyi @@ -17,8 +17,6 @@ _SCIP_BOUNDTYPE_TO_STRING: dict _core_dot: Incomplete _core_dot_2d: Incomplete _core_sum: Incomplete -_expr_richcmp: Incomplete -_is_number: Incomplete buildGenExprObj: Incomplete cos: Incomplete exp: Incomplete diff --git a/tests/test_expr.py b/tests/test_expr.py index 855f47cff..9b8635885 100644 --- a/tests/test_expr.py +++ b/tests/test_expr.py @@ -1,9 +1,10 @@ import math +import numpy as np import pytest -from pyscipopt import Model, sqrt, log, exp, sin, cos -from pyscipopt.scip import Expr, GenExpr, ExprCons, CONST +from pyscipopt import Model, cos, exp, log, sin, sqrt +from pyscipopt.scip import CONST, Expr, ExprCons, GenExpr @pytest.fixture(scope="module") @@ -249,3 +250,116 @@ def test_abs_abs_expr(): # should print abs(x) not abs(abs(x)) assert str(abs(abs(x))) == str(abs(x)) + + +def test_NotImplemented(): + m = Model() + x = m.addVar(name="x") + + with pytest.raises(TypeError): + "y" + x + with pytest.raises(TypeError): + x + "y" + + with pytest.raises(TypeError): + y = "y" + y += x + with pytest.raises(TypeError): + x += "y" + + with pytest.raises(TypeError): + "y" * x + with pytest.raises(TypeError): + x * "y" + + with pytest.raises(TypeError): + "y" / x + with pytest.raises(TypeError): + x / "y" + + with pytest.raises(TypeError): + "1" <= x + with pytest.raises(TypeError): + x >= "1" + with pytest.raises(TypeError): + x >= "1" + with pytest.raises(TypeError): + "1" == x + with pytest.raises(TypeError): + x == "1" + + genexpr = sqrt(x) + + with pytest.raises(TypeError): + "y" + genexpr + with pytest.raises(TypeError): + genexpr + "y" + + with pytest.raises(TypeError): + y = "y" + y += genexpr + with pytest.raises(TypeError): + genexpr += "y" + + with pytest.raises(TypeError): + "y" * genexpr + with pytest.raises(TypeError): + genexpr * "y" + + with pytest.raises(TypeError): + "y" / genexpr + with pytest.raises(TypeError): + genexpr / "y" + + with pytest.raises(TypeError): + "1" <= genexpr + with pytest.raises(TypeError): + genexpr >= "1" + with pytest.raises(TypeError): + genexpr >= "1" + with pytest.raises(TypeError): + "1" == genexpr + with pytest.raises(TypeError): + genexpr == "1" + + # test Expr + GenExpr + assert str(x + genexpr) == "sum(0.0,sqrt(sum(0.0,prod(1.0,x))),prod(1.0,x))" + assert str(genexpr + x) == "sum(0.0,sqrt(sum(0.0,prod(1.0,x))),prod(1.0,x))" + + # test Expr * GenExpr + assert ( + str(x * genexpr) == "prod(1.0,sqrt(sum(0.0,prod(1.0,x))),sum(0.0,prod(1.0,x)))" + ) + + # test Expr + array + a = np.array([1]) + assert str(x + a) == "[Expr({Term(x): 1.0, Term(): 1.0})]" + # test GenExpr + array + assert str(genexpr + a) == "[sum(1.0,sqrt(sum(0.0,prod(1.0,x))))]" + + a = m.addMatrixVar(1) + # test Expr >= array + assert str(x >= a) == "[ExprCons(Expr({Term(x2): 1.0, Term(x): -1.0}), None, 0.0)]" + # test GenExpr >= array + assert ( + str(genexpr >= a) + == "[ExprCons(sum(0.0,prod(-1.0,sqrt(sum(0.0,prod(1.0,x)))),prod(1.0,x2)), None, 0.0)]" + ) + # test Expr <= array + assert str(x <= a) == "[ExprCons(Expr({Term(x2): 1.0, Term(x): -1.0}), 0.0, None)]" + # test GenExpr <= array + assert ( + str(genexpr <= a) + == "[ExprCons(sum(0.0,prod(-1.0,sqrt(sum(0.0,prod(1.0,x)))),prod(1.0,x2)), 0.0, None)]" + ) + # test Expr == array + assert str(x == a) == "[ExprCons(Expr({Term(x2): 1.0, Term(x): -1.0}), 0.0, 0.0)]" + # test GenExpr == array + assert ( + str(genexpr == a) + == "[ExprCons(sum(0.0,prod(-1.0,sqrt(sum(0.0,prod(1.0,x)))),prod(1.0,x2)), 0.0, 0.0)]" + ) + + # test Expr += GenExpr + x += genexpr + assert str(x) == "sum(0.0,sqrt(sum(0.0,prod(1.0,x))),prod(1.0,x))"