Merge pull request #3099 from jsiirola/immutable-expr-args

Enforce expression immutability in `expr.args`

Author: michaelbynum

pyomo/core/expr/numeric_expr.py

@@ -1162,13 +1162,30 @@ def nargs(self):
 
     @property
     def args(self):
-        if len(self._args_) != self._nargs:
-            self._args_ = self._args_[: self._nargs]
-        return self._args_
+        # We unconditionally make a copy of the args to isolate the user
+        # from future possible updates to the underlying list
+        return self._args_[: self._nargs]
 
     def getname(self, *args, **kwds):
         return 'sum'
 
+    def _trunc_append(self, other):
+        _args = self._args_
+        if len(_args) > self._nargs:
+            _args = _args[: self._nargs]
+        _args.append(other)
+        return self.__class__(_args)
+
+    def _trunc_extend(self, other):
+        _args = self._args_
+        if len(_args) > self._nargs:
+            _args = _args[: self._nargs]
+        if len(other._args_) == other._nargs:
+            _args.extend(other._args_)
+        else:
+            _args.extend(other._args_[: other._nargs])
+        return self.__class__(_args)
+
     def _apply_operation(self, result):
         return sum(result)
 
@@ -1821,17 +1838,13 @@ def _add_native_monomial(a, b):
 def _add_native_linear(a, b):
     if not a:
         return b
-    args = b.args
-    args.append(a)
-    return b.__class__(args)
+    return b._trunc_append(a)
 
 
 def _add_native_sum(a, b):
     if not a:
         return b
-    args = b.args
-    args.append(a)
-    return b.__class__(args)
+    return b._trunc_append(a)
 
 
 def _add_native_other(a, b):
@@ -1872,15 +1885,11 @@ def _add_npv_monomial(a, b):
 
 
 def _add_npv_linear(a, b):
-    args = b.args
-    args.append(a)
-    return b.__class__(args)
+    return b._trunc_append(a)
 
 
 def _add_npv_sum(a, b):
-    args = b.args
-    args.append(a)
-    return b.__class__(args)
+    return b._trunc_append(a)
 
 
 def _add_npv_other(a, b):
@@ -1942,19 +1951,15 @@ def _add_param_linear(a, b):
         a = a.value
         if not a:
             return b
-    args = b.args
-    args.append(a)
-    return b.__class__(args)
+    return b._trunc_append(a)
 
 
 def _add_param_sum(a, b):
     if a.is_constant():
         a = value(a)
         if not a:
             return b
-    args = b.args
-    args.append(a)
-    return b.__class__(args)
+    return b._trunc_append(a)
 
 
 def _add_param_other(a, b):
@@ -1999,15 +2004,11 @@ def _add_var_monomial(a, b):
 
 
 def _add_var_linear(a, b):
-    args = b.args
-    args.append(MonomialTermExpression((1, a)))
-    return b.__class__(args)
+    return b._trunc_append(MonomialTermExpression((1, a)))
 
 
 def _add_var_sum(a, b):
-    args = b.args
-    args.append(a)
-    return b.__class__(args)
+    return b._trunc_append(a)
 
 
 def _add_var_other(a, b):
@@ -2046,15 +2047,11 @@ def _add_monomial_monomial(a, b):
 
 
 def _add_monomial_linear(a, b):
-    args = b.args
-    args.append(a)
-    return b.__class__(args)
+    return b._trunc_append(a)
 
 
 def _add_monomial_sum(a, b):
-    args = b.args
-    args.append(a)
-    return b.__class__(args)
+    return b._trunc_append(a)
 
 
 def _add_monomial_other(a, b):
@@ -2069,49 +2066,35 @@ def _add_monomial_other(a, b):
 def _add_linear_native(a, b):
     if not b:
         return a
-    args = a.args
-    args.append(b)
-    return a.__class__(args)
+    return a._trunc_append(b)
 
 
 def _add_linear_npv(a, b):
-    args = a.args
-    args.append(b)
-    return a.__class__(args)
+    return a._trunc_append(b)
 
 
 def _add_linear_param(a, b):
     if b.is_constant():
         b = b.value
         if not b:
             return a
-    args = a.args
-    args.append(b)
-    return a.__class__(args)
+    return a._trunc_append(b)
 
 
 def _add_linear_var(a, b):
-    args = a.args
-    args.append(MonomialTermExpression((1, b)))
-    return a.__class__(args)
+    return a._trunc_append(MonomialTermExpression((1, b)))
 
 
 def _add_linear_monomial(a, b):
-    args = a.args
-    args.append(b)
-    return a.__class__(args)
+    return a._trunc_append(b)
 
 
 def _add_linear_linear(a, b):
-    args = a.args
-    args.extend(b.args)
-    return a.__class__(args)
+    return a._trunc_extend(b)
 
 
 def _add_linear_sum(a, b):
-    args = b.args
-    args.append(a)
-    return b.__class__(args)
+    return b._trunc_append(a)
 
 
 def _add_linear_other(a, b):
@@ -2126,55 +2109,39 @@ def _add_linear_other(a, b):
 def _add_sum_native(a, b):
     if not b:
         return a
-    args = a.args
-    args.append(b)
-    return a.__class__(args)
+    return a._trunc_append(b)
 
 
 def _add_sum_npv(a, b):
-    args = a.args
-    args.append(b)
-    return a.__class__(args)
+    return a._trunc_append(b)
 
 
 def _add_sum_param(a, b):
     if b.is_constant():
         b = b.value
         if not b:
             return a
-    args = a.args
-    args.append(b)
-    return a.__class__(args)
+    return a._trunc_append(b)
 
 
 def _add_sum_var(a, b):
-    args = a.args
-    args.append(b)
-    return a.__class__(args)
+    return a._trunc_append(b)
 
 
 def _add_sum_monomial(a, b):
-    args = a.args
-    args.append(b)
-    return a.__class__(args)
+    return a._trunc_append(b)
 
 
 def _add_sum_linear(a, b):
-    args = a.args
-    args.append(b)
-    return a.__class__(args)
+    return a._trunc_append(b)
 
 
 def _add_sum_sum(a, b):
-    args = a.args
-    args.extend(b.args)
-    return a.__class__(args)
+    return a._trunc_extend(b)
 
 
 def _add_sum_other(a, b):
-    args = a.args
-    args.append(b)
-    return a.__class__(args)
+    return a._trunc_append(b)
 
 
 #
@@ -2213,9 +2180,7 @@ def _add_other_linear(a, b):
 
 
 def _add_other_sum(a, b):
-    args = b.args
-    args.append(a)
-    return b.__class__(args)
+    return b._trunc_append(a)
 
 
 def _add_other_other(a, b):
@@ -2628,8 +2593,8 @@ def _neg_var(a):
 
 
 def _neg_monomial(a):
-    args = a.args
-    return MonomialTermExpression((-args[0], args[1]))
+    coef, var = a.args
+    return MonomialTermExpression((-coef, var))
 
 
 def _neg_sum(a):

pyomo/core/tests/unit/test_derivs.py

@@ -322,6 +322,18 @@ def test_nested_named_expressions(self):
         self.assertAlmostEqual(derivs[m.y], pyo.value(symbolic[m.y]), tol + 3)
         self.assertAlmostEqual(derivs[m.y], approx_deriv(e, m.y), tol)
 
+    def test_linear_exprs_issue_3096(self):
+        m = pyo.ConcreteModel()
+        m.y1 = pyo.Var(initialize=10)
+        m.y2 = pyo.Var(initialize=100)
+        e = (m.y1 - 0.5) * (m.y1 - 0.5) + (m.y2 - 0.5) * (m.y2 - 0.5)
+        derivs = reverse_ad(e)
+        self.assertEqual(derivs[m.y1], 19)
+        self.assertEqual(derivs[m.y2], 199)
+        symbolic = reverse_sd(e)
+        self.assertExpressionsEqual(symbolic[m.y1], m.y1 - 0.5 + m.y1 - 0.5)
+        self.assertExpressionsEqual(symbolic[m.y2], m.y2 - 0.5 + m.y2 - 0.5)
+
 
 class TestDifferentiate(unittest.TestCase):
     @unittest.skipUnless(sympy_available, "test requires sympy")

pyomo/core/tests/unit/test_numeric_expr.py

@@ -1618,9 +1618,9 @@ def test_nestedProduct2(self):
             ),
         )
         # Verify shared args...
-        self.assertIsNot(e1._args_, e2._args_)
-        self.assertIs(e1._args_, e3._args_)
-        self.assertIs(e1._args_, e.arg(1)._args_)
+        self.assertIs(e1._args_, e2._args_)
+        self.assertIsNot(e1._args_, e3._args_)
+        self.assertIs(e1._args_, e.arg(0)._args_)
         self.assertIs(e.arg(0).arg(0), e.arg(1).arg(0))
         self.assertIs(e.arg(0).arg(1), e.arg(1).arg(1))
 

pyomo/core/tests/unit/test_numeric_expr_api.py

@@ -950,7 +950,14 @@ def test_sum(self):
         f = e.create_node_with_local_data(e.args)
         self.assertIsNot(f, e)
         self.assertIs(type(f), type(e))
-        self.assertIs(f.args, e.args)
+        self.assertIsNot(f._args_, e._args_)
+        self.assertIsNot(f.args, e.args)
+
+        f = e.create_node_with_local_data(e._args_)
+        self.assertIsNot(f, e)
+        self.assertIs(type(f), type(e))
+        self.assertIs(f._args_, e._args_)
+        self.assertIsNot(f.args, e.args)
 
         f = e.create_node_with_local_data((m.x, 2, 3))
         self.assertIsNot(f, e)