trying to improve performance for NumericQ/NumberQ tests

adding benchmark

Negative avoids to call `evaluate(evaluation)` by calling the `Less.sapply` method directly.

Positive
  'Positive[Sqrt[2]]'
      100 loops, avg: 19.7 ms, best: 18.6 ms, median: 19.5 ms per loop
  'Positive[Sqrt[-2]]'
      100 loops, avg: 31.5 ms, best: 28.9 ms, median: 29.8 ms per loop
  'Positive[Sqrt[2.]]'
     5000 loops, avg:  949 us, best:  825 us, median:  890 us per loop
  'Positive[Sqrt[-2.]]'
     5000 loops, avg:  956 us, best:  820 us, median:  887 us per loop
  'Positive[q]'
    10000 loops, avg:  164 us, best:  141 us, median:  147 us per loop
  'Positive["q"]'
    10000 loops, avg:  159 us, best:  137 us, median:  143 us per loop

Negative
  'Negative[Sqrt[2]]'
      100 loops, avg: 19.7 ms, best: 18.9 ms, median: 19.5 ms per loop
  'Negative[Sqrt[-2]]'
      100 loops, avg: 31.8 ms, best: 29.2 ms, median: 30.3 ms per loop
  'Negative[Sqrt[2.]]'
     5000 loops, avg:  954 us, best:  836 us, median:  901 us per loop
  'Negative[Sqrt[-2.]]'
     5000 loops, avg:  952 us, best:  832 us, median:  898 us per loop
  'Negative[q]'
    10000 loops, avg:  165 us, best:  145 us, median:  150 us per loop
  'Negative["q"]'
    10000 loops, avg:  159 us, best:  140 us, median:  144 us per loop

NumericQ
  'NumericQ[Sqrt[2]]'
     5000 loops, avg:  987 us, best:  863 us, median:  934 us per loop
  'NumericQ[Sqrt[-2]]'
     1000 loops, avg: 1.68 ms, best: 1.47 ms, median: 1.65 ms per loop
  'NumericQ[Sqrt[2.]]'
     5000 loops, avg:  917 us, best:  802 us, median:  862 us per loop
  'NumericQ[Sqrt[-2.]]'
     5000 loops, avg:  948 us, best:  810 us, median:  878 us per loop
  'NumericQ[q]'
    10000 loops, avg: 81.5 us, best: 71.2 us, median: 73.2 us per loop
  'NumericQ["q"]'
    10000 loops, avg: 77.2 us, best: 67.8 us, median: 69.6 us per loop

fixup: Format Python code with Black

Now `expression.is_numeric` accepts an `evaluation` object as an argument and   `NumericQ` calls directly that method.

Positive
  'Positive[Sqrt[2]]'
      100 loops, avg: 19.4 ms, best: 18.4 ms, median: 19.2 ms per loop
  'Positive[Sqrt[-2]]'
      100 loops, avg: 31.5 ms, best: 28.7 ms, median: 29.8 ms per loop
  'Positive[Sqrt[2.]]'
     5000 loops, avg:  948 us, best:  822 us, median:  890 us per loop
  'Positive[Sqrt[-2.]]'
     5000 loops, avg:  943 us, best:  813 us, median:  882 us per loop
  'Positive[q]'
    10000 loops, avg:  160 us, best:  138 us, median:  144 us per loop
  'Positive["q"]'
    10000 loops, avg:  157 us, best:  133 us, median:  139 us per loop

Negative
  'Negative[Sqrt[2]]'
      100 loops, avg: 19.4 ms, best: 18.3 ms, median: 19.1 ms per loop
  'Negative[Sqrt[-2]]'
      100 loops, avg: 31.5 ms, best: 28.6 ms, median: 29.6 ms per loop
  'Negative[Sqrt[2.]]'
     5000 loops, avg:  951 us, best:  813 us, median:  890 us per loop
  'Negative[Sqrt[-2.]]'
     5000 loops, avg:  928 us, best:  807 us, median:  876 us per loop
  'Negative[q]'
    10000 loops, avg: 87.2 us, best: 75.9 us, median: 78.4 us per loop
  'Negative["q"]'
    10000 loops, avg:   84 us, best: 73.4 us, median: 75.7 us per loop

NumericQ
  'NumericQ[Sqrt[2]]'
     1000 loops, avg: 1.01 ms, best:  866 us, median:  942 us per loop
  'NumericQ[Sqrt[-2]]'
     1000 loops, avg: 1.66 ms, best: 1.45 ms, median: 1.62 ms per loop
  'NumericQ[Sqrt[2.]]'
     5000 loops, avg:  904 us, best:  787 us, median:  851 us per loop
  'NumericQ[Sqrt[-2.]]'
     5000 loops, avg:  912 us, best:  794 us, median:  859 us per loop
  'NumericQ[q]'
    10000 loops, avg:   78 us, best: 69.7 us, median: 71.4 us per loop
  'NumericQ["q"]'
    10000 loops, avg: 75.5 us, best: 66.2 us, median: 68.3 us per loop

listing changes

Author: mmatera

CHANGES.rst

@@ -1,6 +1,18 @@
 CHANGES
 =======
 
+Internals
+=========
+
+* now `Expression.is_numeric()` accepts an `Evaluation` object as a parameter,
+  to use the definitions.
+* To numerify expressions, the function `_numeric_evaluation_with_prec` was introduced in the module `mathics.builtin.numeric` to avoid the idiom
+  `Expression("N", expr, prec).evaluate(evaluation)`. The idea is to avoid when it is possible to call the Pattern matching routines to obtain the numeric value of an expression.
+* `Positive`,  `Negative`, `NonPositive` and `NonNegative` do not check their arguments using the Pattern matching mechanism, but inside the Python code.
+* special cases for `quick_pattern_test` were reduced as much as possible.
+* A bug comming from a failure in the order in which `mathics.core.definitions` stores the rules was fixed.
+
+
 4.0.1
 -----
 

mathics/benchmark.py

@@ -26,6 +26,41 @@
 
 # Mathics expressions to benchmark
 BENCHMARKS = {
+    "NumericQ": [
+        "NumericQ[Sqrt[2]]",
+        "NumericQ[Sqrt[-2]]",
+        "NumericQ[Sqrt[2.]]",
+        "NumericQ[Sqrt[-2.]]",
+        "NumericQ[q]",
+        'NumericQ["q"]',
+    ],
+    # This function checks for numericQ in ints argument before calling
+    "Positive": [
+        "Positive[Sqrt[2]]",
+        "Positive[Sqrt[-2]]",
+        "Positive[Sqrt[2.]]",
+        "Positive[Sqrt[-2.]]",
+        "Positive[q]",
+        'Positive["q"]',
+    ],
+    # This function uses the WL rules definition, like in master
+    "NonNegative": [
+        "NonNegative[Sqrt[2]]",
+        "NonNegative[Sqrt[-2]]",
+        "NonNegative[Sqrt[2.]]",
+        "NonNegative[Sqrt[-2.]]",
+        "NonNegative[q]",
+        'NonNegative["q"]',
+    ],
+    # This function does the check inside the method.
+    "Negative": [
+        "Negative[Sqrt[2]]",
+        "Negative[Sqrt[-2]]",
+        "Negative[Sqrt[2.]]",
+        "Negative[Sqrt[-2.]]",
+        "Negative[q]",
+        'Negative["q"]',
+    ],
     "Arithmetic": ["1 + 2", "5 * 3"],
     "Plot": [
         "Plot[0, {x, -3, 3}]",

mathics/builtin/arithmetic.py

@@ -47,6 +47,7 @@
 from mathics.core.convert import from_sympy, SympyExpression, sympy_symbol_prefix
 from mathics.builtin.scoping import dynamic_scoping
 from mathics.builtin.inference import get_assumptions_list, evaluate_predicate
+from mathics.builtin.numeric import _numeric_evaluation_with_prec
 
 
 @lru_cache(maxsize=1024)
@@ -133,7 +134,7 @@ def apply(self, z, evaluation):
             prec = min_prec(*args)
             d = dps(prec)
             args = [
-                Expression(SymbolN, arg, Integer(d)).evaluate(evaluation)
+                _numeric_evaluation_with_prec(arg, evaluation, Integer(d))
                 for arg in args
             ]
             with mpmath.workprec(prec):

mathics/builtin/base.py

@@ -527,11 +527,13 @@ def __init__(self, *args, **kwargs):
 class Test(Builtin):
     def apply(self, expr, evaluation) -> Symbol:
         "%(name)s[expr_]"
-
-        if self.test(expr):
+        tst = self.test(expr)
+        if tst:
             return SymbolTrue
-        else:
+        elif tst is False:
             return SymbolFalse
+        else:
+            return
 
 
 class SympyFunction(SympyObject):

mathics/builtin/comparison.py

@@ -198,7 +198,7 @@ class _InequalityOperator(BinaryOperator):
     def numerify_args(items, evaluation):
         items_sequence = items.get_sequence()
         all_numeric = all(
-            item.is_numeric() and item.get_precision() is None
+            item.is_numeric(evaluation) and item.get_precision() is None
             for item in items_sequence
         )
 

mathics/builtin/datentime.py

@@ -630,7 +630,7 @@ def apply_any(self, args, evaluation, options):
             timezone = Real(-time.timezone / 3600.0)
         else:
             timezone = options["System`TimeZone"].evaluate(evaluation)
-            if not timezone.is_numeric():
+            if not timezone.is_numeric(evaluation):
                 evaluation.message("DateObject", "notz", timezone)
 
         # TODO: if tz != timezone, shift the datetime list.
@@ -1123,7 +1123,7 @@ def apply_2(self, expr, t, evaluation):
         def apply_3(self, expr, t, failexpr, evaluation):
             "TimeConstrained[expr_, t_, failexpr_]"
             t = t.evaluate(evaluation)
-            if not t.is_numeric():
+            if not t.is_numeric(evaluation):
                 evaluation.message("TimeConstrained", "timc", t)
                 return
             try:

mathics/builtin/inference.py

@@ -156,7 +156,7 @@ def logical_expand_assumptions(assumptions_list, evaluation):
                 evaluation.message("Assumption", "faas")
                 changed = True
                 continue
-            if assumption.is_numeric():
+            if assumption.is_numeric(evaluation):
                 evaluation.message("Assumption", "baas")
                 changed = True
                 continue
@@ -306,7 +306,7 @@ def get_assumption_rules_dispatch(evaluation):
         if pat.has_form("Equal", 2):
             if value:
                 lhs, rhs = pat._leaves
-                if lhs.is_numeric():
+                if lhs.is_numeric(evaluation):
                     assumption_rules.append(Rule(rhs, lhs))
                 else:
                     assumption_rules.append(Rule(lhs, rhs))

mathics/builtin/intfns/divlike.py

@@ -367,7 +367,7 @@ class QuotientRemainder(Builtin):
 
     def apply(self, m, n, evaluation):
         "QuotientRemainder[m_, n_]"
-        if m.is_numeric() and n.is_numeric():
+        if m.is_numeric(evaluation) and n.is_numeric():
             py_m = m.to_python()
             py_n = n.to_python()
             if py_n == 0:

mathics/builtin/lists.py

@@ -2704,7 +2704,7 @@ def convert_vectors(p):
                     raise _IllegalDataPoint
                 yield v
 
-        if dist_p[0].is_numeric():
+        if dist_p[0].is_numeric(evaluation):
             numeric_p = [[x] for x in convert_scalars(dist_p)]
         else:
             numeric_p = list(convert_vectors(dist_p))

mathics/builtin/moments/basic.py

@@ -49,7 +49,7 @@ def apply(self, l, evaluation):
                 return self.rect(l)
             except _NotRectangularException:
                 evaluation.message("Median", "rectn", Expression("Median", l))
-        elif all(leaf.is_numeric() for leaf in l.leaves):
+        elif all(leaf.is_numeric(evaluation) for leaf in l.leaves):
             v = l.get_mutable_leaves()  # copy needed for introselect
             n = len(v)
             if n % 2 == 0:  # even number of elements?

mathics/builtin/numbers/algebra.py

@@ -1476,7 +1476,7 @@ def coeff_power_internal(self, expr, var_exprs, filt, evaluation, form="expr"):
         def key_powers(lst):
             key = Expression("Plus", *lst)
             key = key.evaluate(evaluation)
-            if key.is_numeric():
+            if key.is_numeric(evaluation):
                 return key.to_python()
             return 0
 

mathics/builtin/numbers/calculus.py

@@ -18,7 +18,6 @@
     SymbolTrue,
     SymbolFalse,
     SymbolList,
-    SymbolN,
     SymbolRule,
     SymbolUndefined,
     from_python,
@@ -27,6 +26,7 @@
 from mathics.core.rules import Pattern
 from mathics.core.numbers import dps
 from mathics.builtin.scoping import dynamic_scoping
+from mathics.builtin.numeric import _numeric_evaluation_with_prec
 from mathics import Symbol
 
 import sympy
@@ -1280,9 +1280,8 @@ def sub(evaluation):
         # TODO: use Precision goal...
         if x1 == x0:
             break
-        x0 = Expression(SymbolN, x1).evaluate(
-            evaluation
-        )  # N required due to bug in sympy arithmetic
+        x0 = _numeric_evaluation_with_prec(x1, evaluation)
+        # N required due to bug in sympy arithmetic
         count += 1
     else:
         evaluation.message("FindRoot", "maxiter")
@@ -1377,7 +1376,7 @@ class FindRoot(Builtin):
     def apply(self, f, x, x0, evaluation, options):
         "FindRoot[f_, {x_, x0_}, OptionsPattern[]]"
         # First, determine x0 and x
-        x0 = Expression(SymbolN, x0).evaluate(evaluation)
+        x0 = _numeric_evaluation_with_prec(x0, evaluation)
         if not isinstance(x0, Number):
             evaluation.message("FindRoot", "snum", x0)
             return
@@ -1545,7 +1544,7 @@ def apply_makeboxes(self, x, x0, data, nmin, nmax, den, form, evaluation):
 
         expansion = []
         for i, leaf in enumerate(data.leaves):
-            if leaf.is_numeric() and leaf.is_zero:
+            if leaf.is_numeric(evaluation) and leaf.is_zero:
                 continue
             if powers[i].is_zero:
                 expansion.append(leaf)

mathics/builtin/numbers/numbertheory.py

@@ -16,11 +16,12 @@
     Rational,
     Symbol,
     from_python,
-    SymbolN,
 )
 from mathics.core.convert import from_sympy, SympyPrime
 import mpmath
 
+from mathics.builtin.numeric import _numeric_evaluation_with_prec
+
 
 class ContinuedFraction(SympyFunction):
     """
@@ -412,13 +413,13 @@ def apply(self, n, b, evaluation):
             return expr
 
         if n_sympy.is_constant():
-            temp_n = Expression(SymbolN, n).evaluate(evaluation)
+            temp_n = _numeric_evaluation_with_prec(n, evaluation)
             py_n = temp_n.to_python()
         else:
             return expr
 
         if b_sympy.is_constant():
-            temp_b = Expression(SymbolN, b).evaluate(evaluation)
+            temp_b = _numeric_evaluation_with_prec(b, evaluation)
             py_b = temp_b.to_python()
         else:
             return expr
@@ -443,7 +444,7 @@ def apply_2(self, n, evaluation):
             return expr
         # Handle Input with special cases such as PI and E
         if n_sympy.is_constant():
-            temp_n = Expression(SymbolN, n).evaluate(evaluation)
+            temp_n = _numeric_evaluation_with_prec(n, evaluation)
             py_n = temp_n.to_python()
         else:
             return expr

mathics/builtin/numbers/randomnumbers.py

@@ -251,7 +251,7 @@ class _RandomBase(Builtin):
 
     def _size_to_python(self, domain, size, evaluation):
         is_proper_spec = size.get_head_name() == "System`List" and all(
-            n.is_numeric() for n in size.leaves
+            n.is_numeric(evaluation) for n in size.leaves
         )
 
         py_size = size.to_python() if is_proper_spec else None
@@ -589,7 +589,7 @@ def _weights_to_python(self, weights, evaluation):
         # we need to normalize weights as numpy.rand.randchoice expects this and as we can limit
         # accuracy problems with very large or very small weights by normalizing with sympy
         is_proper_spec = weights.get_head_name() == "System`List" and all(
-            w.is_numeric() for w in weights.leaves
+            w.is_numeric(evaluation) for w in weights.leaves
         )
 
         if (
@@ -599,7 +599,7 @@ def _weights_to_python(self, weights, evaluation):
                 "Divide", weights, Expression("Total", weights)
             ).evaluate(evaluation)
             if norm_weights is None or not all(
-                w.is_numeric() for w in norm_weights.leaves
+                w.is_numeric(evaluation) for w in norm_weights.leaves
             ):
                 return evaluation.message(self.get_name(), "wghtv", weights), None
             weights = norm_weights

mathics/builtin/numeric.py

@@ -42,6 +42,7 @@
     SymbolFalse,
     SymbolTrue,
     SymbolList,
+    SymbolMachinePrecision,
     SymbolN,
     from_python,
 )
@@ -61,6 +62,10 @@ def log_n_b(py_n, py_b) -> int:
     return int(mpmath.ceil(mpmath.log(py_n, py_b))) if py_n != 0 and py_n != 1 else 1
 
 
+def _numeric_evaluation_with_prec(expression, evaluation, prec=SymbolMachinePrecision):
+    return Expression("N", expression, prec).evaluate(evaluation)
+
+
 def _scipy_interface(integrator, options_map, mandatory=None, adapt_func=None):
     """
     This function provides a proxy for scipy.integrate
@@ -1126,7 +1131,7 @@ def apply_with_func_domain(self, func, domain, evaluation, options):
                             (np.arctanh, lambda u: 1.0 / (1.0 - u ** 2))
                         )
                     else:
-                        if not b.is_numeric():
+                        if not b.is_numeric(evaluation):
                             evaluation.message("nlim", coords[i], b)
                             return
                         z = a.leaves[0].value
@@ -1136,7 +1141,7 @@ def apply_with_func_domain(self, func, domain, evaluation, options):
                             (lambda u: b - z + z / u, lambda u: -z * u ** (-2.0))
                         )
                 elif b.get_head_name() == "System`DirectedInfinity":
-                    if not a.is_numeric():
+                    if not a.is_numeric(evaluation):
                         evaluation.message("nlim", coords[i], a)
                         return
                     a = a.value
@@ -1145,14 +1150,14 @@ def apply_with_func_domain(self, func, domain, evaluation, options):
                     coordtransform.append(
                         (lambda u: a - z + z / u, lambda u: z * u ** (-2.0))
                     )
-                elif a.is_numeric() and b.is_numeric():
+                elif a.is_numeric(evaluation) and b.is_numeric(evaluation):
                     a = Expression(SymbolN, a).evaluate(evaluation).value
                     b = Expression(SymbolN, b).evaluate(evaluation).value
                     subdomain2.append([a, b])
                     coordtransform.append(None)
                 else:
                     for x in (a, b):
-                        if not x.is_numeric():
+                        if not x.is_numeric(evaluation):
                             evaluation.message("nlim", coords[i], x)
                     return
 
@@ -1228,17 +1233,7 @@ class NumericQ(Builtin):
 
     def apply(self, expr, evaluation):
         "NumericQ[expr_]"
-
-        def test(expr):
-            if isinstance(expr, Expression):
-                attr = evaluation.definitions.get_attributes(expr.head.get_name())
-                return "System`NumericFunction" in attr and all(
-                    test(leaf) for leaf in expr.leaves
-                )
-            else:
-                return expr.is_numeric()
-
-        return SymbolTrue if test(expr) else SymbolFalse
+        return SymbolTrue if expr.is_numeric(evaluation) else SymbolFalse
 
 
 class Precision(Builtin):
@@ -1648,7 +1643,7 @@ def apply(self, n, evaluation):
         if isinstance(n, Symbol) and n.name.startswith("System`"):
             return evaluation.message("RealDigits", "ndig", n)
 
-        if n.is_numeric():
+        if n.is_numeric(evaluation):
             return self.apply_with_base(n, from_python(10), evaluation)
 
     def apply_with_base(self, n, b, evaluation, nr_elements=None, pos=None):

mathics/builtin/patterns.py

@@ -358,7 +358,7 @@ def apply_list(self, expr, rules, evaluation, options):
             return rules
 
         maxit = self.get_option(options, "MaxIterations", evaluation)
-        if maxit.is_numeric():
+        if maxit.is_numeric(evaluation):
             maxit = maxit.get_int_value()
         else:
             maxit = -1

mathics/builtin/procedural.py

@@ -433,7 +433,7 @@ def apply(self, f, expr, n, evaluation, options):
 
         if count is None:
             count = self.get_option(options, "MaxIterations", evaluation)
-            if count.is_numeric():
+            if count.is_numeric(evaluation):
                 count = count.get_int_value()
             else:
                 count = None

mathics/builtin/recurrence.py

@@ -100,7 +100,7 @@ def is_relation(eqn):
                     left.get_head_name() == func.get_head_name()
                     and len(left.leaves) == 1  # noqa
                     and isinstance(l.leaves[0].to_python(), int)
-                    and r.is_numeric()
+                    and r.is_numeric(evaluation)
                 ):
 
                     r_sympy = r.to_sympy()