Better plot tests (#1541)

We only have a couple plot tests that actually look at the output, and
those were constructed and run laboriously, and none of them test the
new vectorized plot code. This makes refactoring the code risky (and so
far I've been relying mostly on end-to-end testing that compares the
generated visuals in my test front-end).

Here's a different approach: the idea is to write the result expression
to a file in outline tree form, and then compare the actual result with
an expected reference result using diff. This provides easily
constructed tests with a fairly readable indication of what has changed.
See comment in test_plot__detailed.py

In addition, this change
* Makes it possible to change at runtime whether vectorized or classic
plot code is used
* Augments print_expression_tree to print the abbreviated numpy arrays
(default numpy str implementation) so that numeric data can be checked
efficiently for gross changes.

Author: bdlucas1

mathics/builtin/drawing/plot.py

@@ -15,6 +15,8 @@
 
 import palettable
 
+import mathics.eval.drawing.plot3d
+import mathics.eval.drawing.plot3d_vectorized
 from mathics.builtin.drawing.graphics3d import Graphics3D
 from mathics.builtin.graphics import Graphics
 from mathics.builtin.options import options_to_rules
@@ -63,23 +65,25 @@
 # Set option such as $UseVectorizedPlot, and maybe a non-standard Plot3D option.
 # For now an env variable is simplest.
 # TODO: work out exactly how to deploy.
+
+
+# can be set via environment variable at startup time,
+# or changed dynamically by setting the use_vectorized_plot flag
 use_vectorized_plot = os.getenv("MATHICS3_USE_VECTORIZED_PLOT", False)
-if use_vectorized_plot:
-    from mathics.eval.drawing.plot3d_vectorized import (
-        eval_ComplexPlot,
-        eval_ComplexPlot3D,
-        eval_ContourPlot,
-        eval_DensityPlot,
-        eval_Plot3D,
-    )
-else:
-    from mathics.eval.drawing.plot3d import (
-        eval_ComplexPlot,
-        eval_ComplexPlot3D,
-        eval_ContourPlot,
-        eval_DensityPlot,
-        eval_Plot3D,
+
+
+# get the plot eval function for the given class,
+# depending on whether vectorized plot functions are enabled
+def get_plot_eval_function(cls):
+    function_name = "eval_" + cls.__name__
+    plot_module = (
+        mathics.eval.drawing.plot3d_vectorized
+        if use_vectorized_plot
+        else mathics.eval.drawing.plot3d
     )
+    fun = getattr(plot_module, function_name)
+    return fun
+
 
 # This tells documentation how to sort this module
 # Here we are also hiding "drawing" since this erroneously appears at the top level.
@@ -649,7 +653,8 @@ def eval(
             plot_options.functions = [functions]
 
         # subclass must set eval_function and graphics_class
-        graphics = self.eval_function(plot_options, evaluation)
+        eval_function = get_plot_eval_function(self.__class__)
+        graphics = eval_function(plot_options, evaluation)
         if not graphics:
             return
 
@@ -833,7 +838,6 @@ class ComplexPlot3D(_Plot3D):
     options = _Plot3D.options3d | {"Mesh": "None"}
 
     many_functions = True
-    eval_function = staticmethod(eval_ComplexPlot3D)
     graphics_class = Graphics3D
 
 
@@ -857,7 +861,6 @@ class ComplexPlot(_Plot3D):
     options = _Plot3D.options2d
 
     many_functions = False
-    eval_function = staticmethod(eval_ComplexPlot)
     graphics_class = Graphics
 
 
@@ -883,7 +886,6 @@ class ContourPlot(_Plot3D):
     # TODO: other options?
 
     many_functions = True
-    eval_function = staticmethod(eval_ContourPlot)
     graphics_class = Graphics
 
 
@@ -916,7 +918,6 @@ class DensityPlot(_Plot3D):
     options = _Plot3D.options2d
 
     many_functions = False
-    eval_function = staticmethod(eval_DensityPlot)
     graphics_class = Graphics
 
 
@@ -2036,5 +2037,4 @@ class Plot3D(_Plot3D):
     options = _Plot3D.options3d
 
     many_functions = True
-    eval_function = staticmethod(eval_Plot3D)
     graphics_class = Graphics3D

mathics/core/util.py

@@ -3,12 +3,14 @@
 Miscellaneous mathics.core utility functions.
 """
 
+import re
 import sys
 from itertools import chain
 from pathlib import PureWindowsPath
 from platform import python_implementation
 from typing import Optional
 
+from mathics.core.atoms import MachineReal, NumericArray
 from mathics.core.symbols import Symbol
 
 IS_PYPY = python_implementation() == "PyPy"
@@ -117,20 +119,46 @@ def subranges(
             )
 
 
-def print_expression_tree(expr, indent="", marker=lambda expr: ""):
+def print_expression_tree(
+    expr, indent="", marker=lambda expr: "", file=None, approximate=False
+):
     """
     Print a Mathics Expression as an indented tree.
     Caller may supply a marker function that computes a marker
     to be displayed in the tree for the given node.
+    The approximate flag causes numbers to be printed with fewer digits
+    and the number of bits of precision (i.e. Real32 vs Real64) to be
+    omitted from printing for numpy arrays, to faciliate comparisons
+    across systems.
     """
+    if file is None:
+        file = sys.stdout
+
     if isinstance(expr, Symbol):
-        print(f"{indent}{marker(expr)}{expr}")
+        print(f"{indent}{marker(expr)}{expr}", file=file)
     elif not hasattr(expr, "elements"):
-        print(f"{indent}{marker(expr)}{expr.get_head()} {expr}")
+        if isinstance(expr, MachineReal) and approximate:
+            # fewer digits
+            value = str(round(expr.value * 1e6) / 1e6)
+        elif isinstance(expr, NumericArray) and approximate:
+            # Real32/64->Real*, Integer32/64->Integer*
+            value = re.sub("[0-9]+,", "*,", str(expr))
+        else:
+            value = str(expr)
+        print(f"{indent}{marker(expr)}{expr.get_head()} {value}", file=file)
+        if isinstance(expr, NumericArray):
+            # numpy provides an abbreviated version of the array
+            # it is inherently approximated (i.e. limited precision) in its own way
+            na_str = str(expr.value)
+            i = indent + "  "
+            na_str = i + na_str.replace("\n", "\n" + i)
+            print(na_str, file=file)
     else:
-        print(f"{indent}{marker(expr)}{expr.head}")
+        print(f"{indent}{marker(expr)}{expr.head}", file=file)
         for elt in expr.elements:
-            print_expression_tree(elt, indent + "  ", marker=marker)
+            print_expression_tree(
+                elt, indent + "  ", marker=marker, file=file, approximate=approximate
+            )
 
 
 def print_sympy_tree(expr, indent=""):

test/builtin/drawing/test_plot.py

@@ -252,6 +252,10 @@ def test_plot(str_expr, msgs, str_expected, fail_msg):
     )
 
 
+#
+# NOTE: I think the following tests have been superseded by test_plot_detail.py which
+# does similar (actually, more stringent) tests much less laboriously. Keeping these
+# for now just in case, but probably better to add new tests to test_plot_detail.py
 #
 # Call plotting functions and examine the structure of the output
 # In case of error trees are printed with an embedded >>> marker showing location of error

test/builtin/drawing/test_plot_detail.py

@@ -0,0 +1,205 @@
+"""
+These tests evaluate Plot* functions, write the result expression to a file in
+outline tree form, and then compare the actual result with an expected reference
+result using diff. For example, if the code that emits a PlotRange based on
+the actual range of data plotted is disabled, the diff looks like this,
+making it fairly clear what is wrong:
+
+@@ -109,13 +109,7 @@
+     System`None
+   System`Rule
+     System`PlotRange
+-    System`List
+-      System`List
+-        System`Real 0.0
+-        System`Real 1.0
+-      System`List
+-        System`Real 0.0
+-        System`Real 1.0
++    System`Automatic
+   System`Rule
+     System`PlotRangeClipping
+     System`False
+
+The NumericArrays are emitted using NumPy's default str, which is an
+abbreviated display of the array, which has enough data that it should
+generally catch any gross error. For example if the function being
+plotted is changed the diff shows that the the of the array is correct,
+but the xyz coordinates output points are changed:
+
+@@ -7,12 +7,12 @@
+     System`GraphicsComplex
+       System`NumericArray NumericArray[Real64, 40000×3]
+         [[0.         0.         0.        ]
+-         [0.00502513 0.         0.        ]
+-         [0.01005025 0.         0.        ]
++         [0.00502513 0.         0.00502513]
++         [0.01005025 0.         0.01005025]
+          ...
+-         [0.98994975 1.         0.98994975]
+-         [0.99497487 1.         0.99497487]
+-         [1.         1.         1.        ]]
++         [0.98994975 1.         1.98994975]
++         [0.99497487 1.         1.99497487]
++         [1.         1.         2.        ]]
+       System`Polygon
+         System`NumericArray NumericArray[Integer64, 39601×4]
+           [[    1     2   202   201]
+
+The reference results are not huge but they are too unwieldy
+to include in code, so they are stored as files in their own
+*_ref directory.
+"""
+
+import os
+import subprocess
+
+# couple tests depend on ths
+try:
+    import skimage
+except:
+    skimage = None
+
+from test.helper import session
+
+import mathics.builtin.drawing.plot as plot
+from mathics.core.util import print_expression_tree
+
+# common plotting options for 2d plots to test with and without
+opt2 = """
+    AspectRatio -> 2,
+    Axes -> False,
+    Frame -> False,
+    Mesh -> Full,
+    PlotPoints -> 10
+"""
+
+# 3d plots add these options
+opt3 = (
+    opt2
+    + """,
+    BoxRatios -> {1, 2, 3}
+"""
+)
+
+# non-vectorized available, vectorized not available,
+classic = [
+    ("barchart", "BarChart[{3,5,2,7}]", opt2, True),
+    ("discreteplot", "DiscretePlot[n^2,{n,1,10}]", opt2, True),
+    ("histogram", "Histogram[{1,1,1,5,5,7,8,8,8}]", opt2, True),
+    ("listlineplot", "ListLinePlot[{1,4,2,5,3}]", opt2, True),
+    ("listplot", "ListPlot[{1,4,2,5,3}]", opt2, True),
+    ("liststepplot", "ListStepPlot[{1,4,2,5,3}]", opt2, True),
+    # ("manipulate", "Manipulate[Plot[a x,{x,0,1}],{a,0,5}]", opt2, True),
+    ("numberlineplot", "NumberLinePlot[{1,3,4}]", opt2, True),
+    ("parametricplot", "ParametricPlot[{t,2 t},{t,0,2}]", opt2, True),
+    ("piechart", "PieChart[{3,2,5}]", opt2, True),
+    ("plot", "Plot[x, {x, 0, 1}]", opt2, True),
+    ("polarplot", "PolarPlot[3 θ,{θ,0,2}]", opt2, True),
+]
+
+# vectorized available, non-vectorized not available
+vectorized = [
+    ("complexplot", "ComplexPlot[Exp[I z],{z,-2-2 I,2+2 I}]", opt2, True),
+    ("complexplot3d", "ComplexPlot3D[Exp[I z],{z,-2-2 I,2+2 I}]", opt3, True),
+    ("contourplot-1", "ContourPlot[x^2-y^2,{x,-2,2},{y,-2,2}]", opt2, skimage),
+    ("contourplot-2", "ContourPlot[x^2+y^2==1,{x,-2,2},{y,-2,2}]", opt2, skimage),
+]
+
+# both vectorized and non-vectorized available
+both = [
+    ("densityplot", "DensityPlot[x y,{x,-2,2},{y,-2,2}]", opt2, True),
+    ("plot3d", "Plot3D[x y,{x,-2,2},{y,-2,2}]", opt3, True),
+]
+
+# compute reference dir, which is this file minus .py plus _ref
+path, _ = os.path.splitext(__file__)
+ref_dir = path + "_ref"
+print(f"ref_dir {ref_dir}")
+
+
+def one_test(name, str_expr, vec, opt, act_dir="/tmp"):
+    # update name and set use_vectorized_plot depending on
+    # whether vectorized test
+    if vec:
+        name += "-vec"
+        plot.use_vectorized_plot = vec
+    else:
+        name += "-cls"
+
+    # update name and splice in options depending on
+    # whether default or with-options test
+    if opt:
+        name += "-opt"
+        str_expr = f"{str_expr[:-1]}, {opt}]"
+    else:
+        name += "-def"
+
+    print(f"=== running {name} {str_expr}")
+
+    try:
+        # evaluate the expression to be tested
+        expr = session.parse(str_expr)
+        act_expr = expr.evaluate(session.evaluation)
+        if len(session.evaluation.out):
+            print("=== messages:")
+            for message in session.evaluation.out:
+                print(message.text)
+        assert not session.evaluation.out, "no output messages expected"
+
+        # write the results to act_fn in act_dir
+        act_fn = os.path.join(act_dir, f"{name}.txt")
+        with open(act_fn, "w") as act_f:
+            print_expression_tree(act_expr, file=act_f, approximate=True)
+
+        # use diff to compare the actual result in act_fn to reference result in ref_fn,
+        # with a fallback of simple string comparison if diff is not available
+        ref_fn = os.path.join(ref_dir, f"{name}.txt")
+        try:
+            result = subprocess.run(
+                ["diff", "-U", "5", ref_fn, act_fn], capture_output=False
+            )
+            assert result.returncode == 0, "reference and actual result differ"
+        except OSError:
+            with open(ref_fn) as ref_f, open(act_fn) as act_f:
+                ref_str, act_str = ref_f.read(), act_f.read()
+            assert ref_str == act_str, "reference and actual result differ"
+
+        # remove /tmp file if test was successful
+        if act_fn != ref_fn:
+            os.remove(act_fn)
+
+    finally:
+        plot.use_vectorized_plot = False
+
+
+def test_all(act_dir="/tmp", opt=None):
+    # run twice, once without and once with options
+    for use_opt in [False, True]:
+        # run classic tests
+        for name, str_expr, opt, cond in classic + both:
+            if cond:
+                opt = opt if use_opt else None
+                one_test(name, str_expr, False, opt, act_dir)
+
+        # run vectorized tests
+        for name, str_expr, opt, cond in vectorized + both:
+            if cond:
+                opt = opt if use_opt else None
+                one_test(name, str_expr, True, opt, act_dir)
+
+
+if __name__ == "__main__":
+    # reference files can be generated by pointing saved actual
+    # output at reference dir instead of /tmp
+    def make_ref_files():
+        test_all(ref_dir)
+
+    def run_tests():
+        try:
+            test_all()
+        except AssertionError:
+            print("FAIL")
+
+    make_ref_files()
+    # run_tests()