Merge pull request #2948 from jsiirola/named-expr-is-numericvalue

Named expressions: `expr` should always return `NumericValue`

pyomo/contrib/fbbt/fbbt.py

@@ -443,10 +443,11 @@ def _prop_bnds_leaf_to_root_GeneralExpression(node, bnds_dict, feasibility_tol):
         region is removed due to floating point arithmetic and to prevent math domain errors (a larger value
         is more conservative).
     """
-    if node.expr.__class__ in native_types:
-        expr_lb = expr_ub = node.expr
+    (expr,) = node.args
+    if expr.__class__ in native_types:
+        expr_lb = expr_ub = expr
     else:
-        expr_lb, expr_ub = bnds_dict[node.expr]
+        expr_lb, expr_ub = bnds_dict[expr]
     bnds_dict[node] = (expr_lb, expr_ub)
 
 

pyomo/core/base/expression.py

@@ -59,10 +59,11 @@ class _ExpressionData(numeric_expr.NumericValue):
 
     def __call__(self, exception=True):
         """Compute the value of this expression."""
-        if self.expr.__class__ in native_types:
+        (arg,) = self._args_
+        if arg.__class__ in native_types:
             # Note: native_types includes NoneType
-            return self.expr
-        return self.expr(exception=exception)
+            return arg
+        return arg(exception=exception)
 
     def is_named_expression_type(self):
         """A boolean indicating whether this in a named expression."""
@@ -73,25 +74,21 @@ def is_expression_type(self, expression_system=None):
         return expression_system is None or expression_system == self.EXPRESSION_SYSTEM
 
     def arg(self, index):
-        if index < 0 or index >= 1:
+        if index != 0:
             raise KeyError("Invalid index for expression argument: %d" % index)
-        return self.expr
-
-    @property
-    def _args_(self):
-        return (self.expr,)
+        return self._args_[0]
 
     @property
     def args(self):
-        return (self.expr,)
+        return self._args_
 
     def nargs(self):
         return 1
 
     def _to_string(self, values, verbose, smap):
         if verbose:
             return "%s{%s}" % (str(self), values[0])
-        if self.expr is None:
+        if self._args_[0] is None:
             return "%s{None}" % str(self)
         return values[0]
 
@@ -106,8 +103,8 @@ def _apply_operation(self, result):
 
     def polynomial_degree(self):
         """A tuple of subexpressions involved in this expressions operation."""
-        if self.expr.__class__ in native_types:
-            return 0
+        if self._args_[0] is None:
+            return None
         return self.expr.polynomial_degree()
 
     def _compute_polynomial_degree(self, result):
@@ -122,8 +119,14 @@ def _is_fixed(self, values):
 
     @property
     def expr(self):
-        """Return expression on this expression."""
-        raise NotImplementedError
+        (arg,) = self._args_
+        if arg is None:
+            return None
+        return as_numeric(arg)
+
+    @expr.setter
+    def expr(self, value):
+        self.set_value(value)
 
     def set_value(self, expr):
         """Set the expression on this expression."""
@@ -158,7 +161,7 @@ class _GeneralExpressionDataImpl(_ExpressionData):
     __slots__ = ()
 
     def __init__(self, expr=None):
-        self._expr = as_numeric(expr) if (expr is not None) else None
+        self._args_ = (expr,)
 
     def create_node_with_local_data(self, values):
         """
@@ -170,34 +173,25 @@ def create_node_with_local_data(self, values):
         """
         obj = ScalarExpression()
         obj.construct()
-        obj.expr = values[0]
+        obj._args_ = values
         return obj
 
     #
     # Abstract Interface
     #
 
-    @property
-    def expr(self):
-        """Return expression on this expression."""
-        return self._expr
-
-    @expr.setter
-    def expr(self, expr):
-        self.set_value(expr)
-
     def set_value(self, expr):
         """Set the expression on this expression."""
         if expr is None or expr.__class__ in native_numeric_types:
-            self._expr = expr
+            self._args_ = (expr,)
             return
         try:
             if expr.is_numeric_type():
-                self._expr = expr
+                self._args_ = (expr,)
                 return
         except AttributeError:
             if check_if_numeric_type(expr):
-                self._expr = expr
+                self._args_ = (expr,)
                 return
         raise ValueError(
             f"Cannot assign {expr.__class__.__name__} to "
@@ -213,33 +207,34 @@ def is_constant(self):
 
     def is_fixed(self):
         """A boolean indicating whether this expression is fixed."""
-        return self._expr.__class__ in native_types or self._expr.is_fixed()
+        (e,) = self._args_
+        return e.__class__ in native_types or e.is_fixed()
 
     # Override the in-place operators here so that we can redirect the
     # dispatcher based on the current contained expression type and not
     # this Expression object (which would map to "other")
 
     def __iadd__(self, other):
-        e = self._expr
+        (e,) = self._args_
         return numeric_expr._add_dispatcher[e.__class__, other.__class__](e, other)
 
     # Note: the default implementation of __isub__ leverages __iadd__
     # and doesn't need to be reimplemented here
 
     def __imul__(self, other):
-        e = self._expr
+        (e,) = self._args_
         return numeric_expr._mul_dispatcher[e.__class__, other.__class__](e, other)
 
     def __idiv__(self, other):
-        e = self._expr
+        (e,) = self._args_
         return numeric_expr._div_dispatcher[e.__class__, other.__class__](e, other)
 
     def __itruediv__(self, other):
-        e = self._expr
+        (e,) = self._args_
         return numeric_expr._div_dispatcher[e.__class__, other.__class__](e, other)
 
     def __ipow__(self, other):
-        e = self._expr
+        (e,) = self._args_
         return numeric_expr._pow_dispatcher[e.__class__, other.__class__](e, other)
 
 
@@ -258,7 +253,7 @@ class _GeneralExpressionData(_GeneralExpressionDataImpl, ComponentData):
         _component  The expression component.
     """
 
-    __slots__ = ('_expr',)
+    __slots__ = ('_args_',)
 
     def __init__(self, expr=None, component=None):
         _GeneralExpressionDataImpl.__init__(self, expr)
@@ -418,13 +413,23 @@ def __init__(self, *args, **kwds):
     # construction
     #
 
+    def __call__(self, exception=True):
+        """Return expression on this expression."""
+        if self._constructed:
+            return super().__call__(exception)
+        raise ValueError(
+            "Evaluating the expression of Expression '%s' "
+            "before the Expression has been constructed (there "
+            "is currently no value to return)." % (self.name)
+        )
+
     @property
     def expr(self):
         """Return expression on this expression."""
         if self._constructed:
             return _GeneralExpressionData.expr.fget(self)
         raise ValueError(
-            "Accessing the expression of expression '%s' "
+            "Accessing the expression of Expression '%s' "
             "before the Expression has been constructed (there "
             "is currently no value to return)." % (self.name)
         )
@@ -442,7 +447,7 @@ def set_value(self, expr):
         if self._constructed:
             return _GeneralExpressionData.set_value(self, expr)
         raise ValueError(
-            "Setting the expression of expression '%s' "
+            "Setting the expression of Expression '%s' "
             "before the Expression has been constructed (there "
             "is currently no object to set)." % (self.name)
         )
@@ -452,7 +457,7 @@ def is_constant(self):
         if self._constructed:
             return _GeneralExpressionData.is_constant(self)
         raise ValueError(
-            "Accessing the is_constant flag of expression '%s' "
+            "Accessing the is_constant flag of Expression '%s' "
             "before the Expression has been constructed (there "
             "is currently no value to return)." % (self.name)
         )
@@ -462,7 +467,7 @@ def is_fixed(self):
         if self._constructed:
             return _GeneralExpressionData.is_fixed(self)
         raise ValueError(
-            "Accessing the is_fixed flag of expression '%s' "
+            "Accessing the is_fixed flag of Expression '%s' "
             "before the Expression has been constructed (there "
             "is currently no value to return)." % (self.name)
         )

pyomo/core/base/objective.py

@@ -154,7 +154,7 @@ class _GeneralObjectiveData(
         _active         A boolean that indicates whether this data is active
     """
 
-    __slots__ = ("_sense", "_expr")
+    __slots__ = ("_sense", "_args_")
 
     def __init__(self, expr=None, sense=minimize, component=None):
         _GeneralExpressionDataImpl.__init__(self, expr)
@@ -271,20 +271,7 @@ def __init__(
 
     def __init__(self, *args, **kwargs):
         _sense = kwargs.pop('sense', minimize)
-        _init = tuple(
-            _arg
-            for _arg in (kwargs.pop('rule', None), kwargs.pop('expr', None))
-            if _arg is not None
-        )
-        if len(_init) == 1:
-            _init = _init[0]
-        elif not _init:
-            _init = None
-        else:
-            raise ValueError(
-                "Duplicate initialization: Objective() only "
-                "accepts one of 'rule=' and 'expr='"
-            )
+        _init = self._pop_from_kwargs('Objective', kwargs, ('rule', 'expr'), None)
 
         kwargs.setdefault('ctype', Objective)
         ActiveIndexedComponent.__init__(self, *args, **kwargs)
@@ -427,6 +414,22 @@ def __init__(self, *args, **kwd):
     # construction
     #
 
+    def __call__(self, exception=True):
+        if self._constructed:
+            if len(self._data) == 0:
+                raise ValueError(
+                    "Evaluating the expression of ScalarObjective "
+                    "'%s' before the Objective has been assigned "
+                    "a sense or expression (there is currently "
+                    "no value to return)." % (self.name)
+                )
+            return super().__call__(exception)
+        raise ValueError(
+            "Evaluating the expression of objective '%s' "
+            "before the Objective has been constructed (there "
+            "is currently no value to return)." % (self.name)
+        )
+
     @property
     def expr(self):
         """Access the expression of this objective."""
@@ -435,8 +438,8 @@ def expr(self):
                 raise ValueError(
                     "Accessing the expression of ScalarObjective "
                     "'%s' before the Objective has been assigned "
-                    "a sense or expression. There is currently "
-                    "nothing to access." % (self.name)
+                    "a sense or expression (there is currently "
+                    "no value to return)." % (self.name)
                 )
             return _GeneralObjectiveData.expr.fget(self)
         raise ValueError(
@@ -458,8 +461,8 @@ def sense(self):
                 raise ValueError(
                     "Accessing the sense of ScalarObjective "
                     "'%s' before the Objective has been assigned "
-                    "a sense or expression. There is currently "
-                    "nothing to access." % (self.name)
+                    "a sense or expression (there is currently "
+                    "no value to return)." % (self.name)
                 )
             return _GeneralObjectiveData.sense.fget(self)
         raise ValueError(