Merge pull request #2493 from jsiirola/expr-inplace

Resolve expression loops with inplace operators on Expression objects

Author: blnicho

pyomo/core/base/expression.py

@@ -195,7 +195,21 @@ def expr(self, expr):
 
     def set_value(self, expr):
         """Set the expression on this expression."""
-        self._expr = as_numeric(expr) if (expr is not None) else None
+        if expr is None:
+            self._expr = None
+            return
+        expr = as_numeric(expr)
+        # In-place operators will leave self as an argument.  We need to
+        # replace that with the current expression in order to avoid
+        # loops in the expression tree.
+        if expr.is_expression_type():
+            _args = expr.args
+            if any(arg is self for arg in _args):
+                new_args = _args.__class__(
+                    arg.expr if arg is self else arg for arg in _args
+                )
+                expr = expr.create_node_with_local_data(new_args)
+        self._expr = expr
 
     def is_constant(self):
         """A boolean indicating whether this expression is constant."""

pyomo/core/tests/unit/test_expression.py

@@ -17,8 +17,12 @@
 
 import pyomo.common.unittest as unittest
 
-from pyomo.environ import ConcreteModel, AbstractModel, Expression, Var, Set, Param, Objective, value, sum_product
+from pyomo.environ import (
+    ConcreteModel, AbstractModel, Expression, Var, Set, Param, Objective,
+    value, sum_product,
+)
 from pyomo.core.base.expression import _GeneralExpressionData
+from pyomo.core.expr.compare import compare_expressions
 from pyomo.common.tee import capture_output
 
 class TestExpressionData(unittest.TestCase):
@@ -902,6 +906,13 @@ def test_iadd(self):
             expr += v
         self.assertEqual(e.expr, 1)
         self.assertEqual(expr(), 2)
+        # Make sure that using in-place operators on named expressions
+        # do not create loops inthe expression tree (test #1890)
+        m.x = Var()
+        m.y = Var()
+        m.e.expr = m.x
+        m.e += m.y
+        self.assertTrue(compare_expressions(m.e.expr, m.x + m.y))
 
     def test_isub(self):
         # make sure simple for loops that look like they
@@ -919,6 +930,13 @@ def test_isub(self):
             expr -= v
         self.assertEqual(e.expr, 1)
         self.assertEqual(expr(), -2)
+        # Make sure that using in-place operators on named expressions
+        # do not create loops inthe expression tree (test #1890)
+        m.x = Var()
+        m.y = Var()
+        m.e.expr = m.x
+        m.e -= m.y
+        self.assertTrue(compare_expressions(m.e.expr, m.x - m.y))
 
     def test_imul(self):
         # make sure simple for loops that look like they
@@ -936,6 +954,13 @@ def test_imul(self):
             expr *= v
         self.assertEqual(e.expr, 3)
         self.assertEqual(expr(), 6)
+        # Make sure that using in-place operators on named expressions
+        # do not create loops inthe expression tree (test #1890)
+        m.x = Var()
+        m.y = Var()
+        m.e.expr = m.x
+        m.e *= m.y
+        self.assertTrue(compare_expressions(m.e.expr, m.x * m.y))
 
     def test_idiv(self):
         # make sure simple for loops that look like they
@@ -968,6 +993,13 @@ def test_idiv(self):
             expr /= v
         self.assertEqual(e.expr, 3)
         self.assertEqual(expr(), 1.5)
+        # Make sure that using in-place operators on named expressions
+        # do not create loops inthe expression tree (test #1890)
+        m.x = Var()
+        m.y = Var()
+        m.e.expr = m.x
+        m.e /= m.y
+        self.assertTrue(compare_expressions(m.e.expr, m.x / m.y))
 
     def test_ipow(self):
         # make sure simple for loops that look like they
@@ -985,6 +1017,13 @@ def test_ipow(self):
             expr **= v
         self.assertEqual(e.expr, 3)
         self.assertEqual(expr(), 9)
+        # Make sure that using in-place operators on named expressions
+        # do not create loops inthe expression tree (test #1890)
+        m.x = Var()
+        m.y = Var()
+        m.e.expr = m.x
+        m.e **= m.y
+        self.assertTrue(compare_expressions(m.e.expr, m.x ** m.y))
 
 if __name__ == "__main__":
     unittest.main()