Merge polecat/quartz-moinn85r (dsc-wmq)

Author: ctessum-claude

python/src/earthsci_discretizations/rules/__init__.py

@@ -8,11 +8,13 @@
 
 Cross-binding contract:
 
-* :func:`eval_coeff` matches the Julia ``EarthSciDiscretizations.eval_coeff``
-  passthrough (which delegates to ``EarthSciSerialization.evaluate``) for
-  every op the rule catalog actually uses today: ``+ - * / ^``, the unary
-  function set (``sin cos tan exp log sqrt abs``), and the constants
-  ``pi`` / ``e``. Unbound variables raise :class:`UnboundVariableError`.
+* :func:`eval_coeff` is a thin adapter over
+  ``earthsci_toolkit.evaluate`` (ESS): JSON dict nodes are converted to
+  the typed :class:`earthsci_toolkit.ExprNode` form once, then ESS's
+  evaluator returns the scalar value. The Julia binding's
+  ``EarthSciDiscretizations.eval_coeff`` does the same passthrough to
+  ``EarthSciSerialization.evaluate``. Unbound variables raise
+  :class:`ValueError` (propagated from ESS).
 * :func:`load_rule` reads a rule JSON file and returns a :class:`Rule`.
   Single-stencil rules expose entries via :attr:`Rule.stencil`;
   multi-stencil rules (PPM-style; ESS §7.5) expose them via
@@ -35,15 +37,14 @@
     reference_samples,
     resolve_sub_stencil,
 )
-from .evaluator import UnboundVariableError, eval_coeff
+from .evaluator import eval_coeff
 from .loader import Rule, StencilEntry, load_rule
 from .stencil import apply_stencil_latlon
 
 __all__ = [
     "OUTPUT_KINDS",
     "Rule",
     "StencilEntry",
-    "UnboundVariableError",
     "apply_stencil_latlon",
     "apply_stencil_periodic_1d",
     "eval_coeff",

python/src/earthsci_discretizations/rules/evaluator.py

@@ -1,165 +1,61 @@
-"""ExpressionNode AST evaluator (Python port of the ESS evaluator surface).
-
-Mirrors ``EarthSciSerialization.evaluate`` (see
-``EarthSciSerialization.jl/src/expression.jl``) for the operator set used
-by the discretization rule catalog. The bindings match
-``src/rule_eval.jl`` in the Julia binding: ``eval_coeff(node, bindings)``
-takes a JSON-decoded AST node (number / variable string / op-dict) and
-returns a Python ``float``.
-
-Op coverage (kept in sync with the Julia ``evaluate`` path):
-
-* arithmetic: ``+ - * / ^``
-* unary functions: ``sin cos tan exp log sqrt abs``
-* constants (zero-arg ops): ``pi`` (alias ``π``), ``e``
-
-Unbound variables raise :class:`UnboundVariableError`. Domain errors on
-``log`` / ``sqrt`` raise :class:`ValueError` to match the ESS behaviour
-where ``DomainError`` propagates out of ``evaluate``.
-
-Ops that the catalog does not use yet (``fn``, ``const``, ``enum``, ``call``,
-slope-ratio ``max``/``min`` until ESS lands them) are deliberately rejected
-so that an unsupported op surfaces immediately rather than producing a
-silent wrong answer.
+"""Coefficient evaluator (thin adapter over ``earthsci_toolkit.evaluate``).
+
+The discretization rule catalog stores coefficients as JSON-decoded
+``ExpressionNode`` ASTs (``int`` / ``float`` / variable name / dict). ESS
+``earthsci_toolkit.evaluate`` consumes the typed
+:class:`earthsci_toolkit.ExprNode` form, so this module converts the dict
+shape to ``ExprNode`` once and delegates — there is no parallel
+operator-dispatch table here. The Julia binding does the same in
+``src/rule_eval.jl``.
 """
 
 from __future__ import annotations
 
-import math
 from collections.abc import Mapping
 from typing import Any
 
-__all__ = ["UnboundVariableError", "eval_coeff"]
-
-
-class UnboundVariableError(KeyError):
-    """Raised when an AST references a variable not present in ``bindings``."""
-
-    def __init__(self, name: str) -> None:
-        super().__init__(name)
-        self.variable_name = name
-
-    def __str__(self) -> str:
-        return f"UnboundVariableError: variable {self.variable_name!r} not found in bindings"
+from earthsci_toolkit.esm_types import ExprNode
 
+from earthsci_toolkit import evaluate
 
-_UNARY_FUNCS: dict[str, Any] = {
-    "sin": math.sin,
-    "cos": math.cos,
-    "tan": math.tan,
-    "exp": math.exp,
-    "abs": abs,
-}
+__all__ = ["eval_coeff"]
 
-_CONSTANTS: dict[str, float] = {
-    "pi": math.pi,
-    "π": math.pi,
-    "e": math.e,
-}
 
+_EXPR_NODE_FIELDS = {f for f in ExprNode.__dataclass_fields__ if f != "args"}
 
-def eval_coeff(node: Any, bindings: Mapping[str, float]) -> float:
-    """Evaluate a JSON-decoded ExpressionNode against scalar bindings.
-
-    Parameters
-    ----------
-    node:
-        ``int`` / ``float`` literal, ``str`` variable name, or ``dict``
-        with ``"op"`` and ``"args"`` keys (the JSON form produced by
-        ``json.load`` on a rule file's ``"coeff"`` field).
-    bindings:
-        Mapping of variable name to numeric value. Values are coerced to
-        ``float``.
-
-    Returns
-    -------
-    float
-        The numeric value of the AST.
-    """
 
+def _to_expr(node: Any) -> Any:
     if isinstance(node, bool):
-        # ``bool`` subclasses ``int`` in Python; reject explicitly to match
-        # the ESS parser, which rejects boolean literals.
+        # ``bool`` subclasses ``int``; reject so boolean literals don't sneak
+        # through as 0/1 (mirrors ESS parser behaviour).
         raise ValueError("Boolean literal is not a valid expression node")
-    if isinstance(node, (int, float)):
-        return float(node)
-    if isinstance(node, str):
-        try:
-            return float(bindings[node])
-        except KeyError as exc:
-            raise UnboundVariableError(node) from exc
+    if isinstance(node, (int, float, str)):
+        return node
     if isinstance(node, Mapping):
         op = node.get("op")
-        if op is None:
+        if not isinstance(op, str):
             raise ValueError(f"AST dict node missing 'op' key: {node!r}")
-        return _eval_op(str(op), node, bindings)
+        kwargs: dict[str, Any] = {"op": op}
+        raw_args = node.get("args")
+        if raw_args is not None:
+            kwargs["args"] = [_to_expr(a) for a in raw_args]
+        for k, v in node.items():
+            if k in {"op", "args"}:
+                continue
+            if k in _EXPR_NODE_FIELDS:
+                kwargs[k] = v
+        return ExprNode(**kwargs)
     raise TypeError(
         f"Invalid AST node type {type(node).__name__}; expected number, string, or mapping"
     )
 
 
-def _eval_op(op: str, node: Mapping[str, Any], bindings: Mapping[str, float]) -> float:
-    # Constants are the only zero-arg ops the catalog uses today; check
-    # before walking ``args`` so a malformed args list doesn't shadow the
-    # zero-arg branch.
-    if op in _CONSTANTS:
-        args = node.get("args") or []
-        if len(args) != 0:
-            raise ValueError(f"{op!r} constant takes no arguments, got {len(args)}")
-        return _CONSTANTS[op]
-
-    raw_args = node.get("args")
-    if raw_args is None:
-        raise ValueError(f"AST op {op!r} missing 'args'")
-    args = [eval_coeff(a, bindings) for a in raw_args]
-    n = len(args)
+def eval_coeff(node: Any, bindings: Mapping[str, float]) -> float:
+    """Evaluate a JSON-decoded ExpressionNode against scalar bindings.
 
-    if op == "+":
-        if n == 1:
-            return args[0]
-        if n == 0:
-            return 0.0
-        return math.fsum(args)
-    if op == "-":
-        if n == 1:
-            return -args[0]
-        if n == 2:
-            return args[0] - args[1]
-        raise ValueError(f"Subtraction requires 1 or 2 arguments, got {n}")
-    if op == "*":
-        if n == 0:
-            return 1.0
-        if n == 1:
-            return args[0]
-        product = 1.0
-        for v in args:
-            product *= v
-        return product
-    if op == "/":
-        if n != 2:
-            raise ValueError(f"Division requires exactly 2 arguments, got {n}")
-        if args[1] == 0.0:
-            raise ZeroDivisionError("division by zero in AST evaluation")
-        return args[0] / args[1]
-    if op == "^":
-        if n != 2:
-            raise ValueError(f"Exponentiation requires exactly 2 arguments, got {n}")
-        return args[0] ** args[1]
-    if op in _UNARY_FUNCS:
-        if n != 1:
-            raise ValueError(f"{op!r} requires exactly 1 argument, got {n}")
-        return _UNARY_FUNCS[op](args[0])
-    if op == "log":
-        if n != 1:
-            raise ValueError(f"'log' requires exactly 1 argument, got {n}")
-        if args[0] <= 0.0:
-            raise ValueError(f"log argument must be positive, got {args[0]}")
-        return math.log(args[0])
-    if op == "sqrt":
-        if n != 1:
-            raise ValueError(f"'sqrt' requires exactly 1 argument, got {n}")
-        if args[0] < 0.0:
-            raise ValueError(f"sqrt argument must be non-negative, got {args[0]}")
-        return math.sqrt(args[0])
+    Returns the numeric value of the AST as a ``float``. Unbound variables
+    raise :class:`ValueError` (propagated from
+    :func:`earthsci_toolkit.evaluate`).
+    """
 
-    raise ValueError(f"Unsupported AST operator: {op!r}")
+    return float(evaluate(_to_expr(node), dict(bindings)))

python/tests/test_centered_2nd_uniform_latlon_rule.py

@@ -118,10 +118,8 @@ def test_stencil_application(rule, fixtures, golden):
 
 
 def test_eval_coeff_unbound_variable_raises(rule):
-    from earthsci_discretizations.rules import UnboundVariableError
-
     entry = rule.stencil[0]
-    with pytest.raises(UnboundVariableError):
+    with pytest.raises(ValueError, match="[Uu]nbound variable"):
         eval_coeff(entry.coeff, {"R": 1.0, "dlon": 0.1})  # missing cos_lat