Fix versioning of empty __props__ in CLinker

aesara/link/c/basic.py

@@ -1461,8 +1461,12 @@ class CLinker(Linker):
         for node_pos, node in enumerate(order):
             if hasattr(node.op, "c_code_cache_version_apply"):
                 version.append(node.op.c_code_cache_version_apply(node))
-            if hasattr(node.op, "__props__"):
-                version.append(node.op.__props__)
+
+            props = getattr(node.op, "__props__", None)
+
+            if props:
+                version.append(props)
+
             for i in node.inputs:
                 if isinstance(i.type, CLinkerObject):
                     version.append(i.type.c_code_cache_version())

conftest.py

@@ -7,7 +7,7 @@ def pytest_sessionstart(session):
     os.environ["AESARA_FLAGS"] = ",".join(
         [
             os.environ.setdefault("AESARA_FLAGS", ""),
-            "warn__ignore_bug_before=all,on_opt_error=raise,on_shape_error=raise",
+            "warn__ignore_bug_before=all,on_opt_error=raise,on_shape_error=raise,cmodule__warn_no_version=True",
         ]
     )
 

tests/link/c/test_basic.py

@@ -455,3 +455,43 @@ def test_shared_input_output():
     gv0 = gv(0)
     assert np.all(fv0 == 5), fv0
     assert np.all(gv0 == 5), gv0
+
+
+def test_cmodule_key_empty_props():
+    """Make sure `CLinker.cmodule_key_` is correct when `COp.__props__` is empty."""
+
+    class MyAdd(COp):
+        __props__ = ()
+
+        def make_node(self, *inputs):
+            inputs = list(map(as_variable, inputs))
+            outputs = [tdouble()]
+            return Apply(self, inputs, outputs)
+
+        def __str__(self):
+            return self.name
+
+        def perform(self, node, inputs, out_):
+            (out,) = out_
+            out[0] = sum(*inputs)
+
+        def c_code_cache_version(self):
+            return (1,)
+
+        def c_code(self, node, name, inp, out, sub):
+            x, y = inp
+            (z,) = out
+            return f"{z} = {x} + {y};"
+
+    x = tdouble("x")
+    y = tdouble("y")
+
+    z = MyAdd()(x, y)
+
+    fg = FunctionGraph(outputs=[z])
+
+    linker = CLinker()
+    linker.accept(fg)
+    key = linker.cmodule_key()
+    # None of the C version values should be empty
+    assert all(kv for kv in key[0])

tests/link/c/test_cmodule.py

@@ -18,9 +18,13 @@ import aesara.tensor as at
 from aesara.compile.function import function
 from aesara.compile.ops import DeepCopyOp
 from aesara.configdefaults import config
-from aesara.link.c.cmodule import GCC_compiler, default_blas_ldflags
+from aesara.graph.basic import Apply
+from aesara.graph.fg import FunctionGraph
+from aesara.link.c.basic import CLinker
+from aesara.link.c.cmodule import GCC_compiler, ModuleCache, default_blas_ldflags
 from aesara.link.c.exceptions import CompileError
-from aesara.tensor.type import dvectors
+from aesara.link.c.op import COp
+from aesara.tensor.type import dvectors, vector
 
 
 class MyOp(DeepCopyOp):
@@ -43,20 +47,47 @@ class MyOp(DeepCopyOp):
         return super(DeepCopyOp, self).c_code(node, name, inames, onames, sub)
 
 
+class MyAdd(COp):
+    __props__ = ()
+
+    def make_node(self, *inputs):
+        outputs = [vector()]
+        return Apply(self, inputs, outputs)
+
+    def perform(self, node, inputs, out_):
+        (out,) = out_
+        out[0] = inputs[0][0] + 1
+
+    def c_code(self, node, name, inp, out, sub):
+        (x,) = inp
+        (z,) = out
+        return f"{z} = {x} + 1;"
+
+
+class MyAddVersioned(MyAdd):
+    def c_code_cache_version(self):
+        return (1,)
+
+
 def test_compiler_error():
     with pytest.raises(CompileError), tempfile.TemporaryDirectory() as dir_name:
         GCC_compiler.compile_str("module_name", "blah", location=dir_name)
 
 
 def test_inter_process_cache():
-    # When an op with c_code, but no version. If we have 2 apply node
-    # in the graph with different inputs variable(so they don't get
-    # merged) but the inputs variable have the same type, do we reuse
-    # the same module? Even if they would generate different c_code?
-    # Currently this test show that we generate the c_code only once.
-    #
-    # This is to know if the c_code can add information specific to the
-    # node.inputs[*].owner like the name of the variable.
+    """
+    TODO FIXME: This explanation is very poorly written.
+
+    When a `COp` with `COp.c_code`, but no version. If we have two `Apply`
+    nodes in a graph with distinct inputs variable, but the input variables
+    have the same `Type`, do we reuse the same module? Even if they would
+    generate different `COp.c_code`?  Currently this test show that we generate
+    the `COp.c_code` only once.
+
+    This is to know if the `COp.c_code` can add information specific to the
+    ``node.inputs[*].owner`` like the name of the variable.
+
+    """
 
     x, y = dvectors("xy")
     f = function([x, y], [MyOp()(x), MyOp()(y)])
@@ -76,12 +107,58 @@ def test_inter_process_cache():
         assert MyOp.nb_called == 1
 
 
+@pytest.mark.filterwarnings("error")
+def test_cache_versioning():
+    """Make sure `ModuleCache._add_to_cache` is working."""
+
+    my_add = MyAdd()
+    with pytest.warns(match=".*specifies no C code cache version.*"):
+        assert my_add.c_code_cache_version() == ()
+
+    my_add_ver = MyAddVersioned()
+    assert my_add_ver.c_code_cache_version() == (1,)
+
+    assert len(MyOp.__props__) == 0
+    assert len(MyAddVersioned.__props__) == 0
+
+    x = vector("x")
+
+    z = my_add(x)
+    z_v = my_add_ver(x)
+
+    with tempfile.TemporaryDirectory() as dir_name:
+        cache = ModuleCache(dir_name)
+
+        lnk = CLinker().accept(FunctionGraph(outputs=[z]))
+        with pytest.warns(match=".*specifies no C code cache version.*"):
+            key = lnk.cmodule_key()
+        assert key[0] == ()
+
+        with pytest.warns(match=".*c_code_cache_version.*"):
+            cache.module_from_key(key, lnk)
+
+        lnk_v = CLinker().accept(FunctionGraph(outputs=[z_v]))
+        key_v = lnk_v.cmodule_key()
+        assert len(key_v[0]) > 0
+
+        assert key_v not in cache.entry_from_key
+
+        stats_before = cache.stats[2]
+        cache.module_from_key(key_v, lnk_v)
+        assert stats_before < cache.stats[2]
+
+
 def test_flag_detection():
-    # Check that the code detecting blas flags does not raise any exception.
-    # It used to happen on python 3 because of improper string handling,
-    # but was not detected because that path is not usually taken,
-    # so we test it here directly.
-    GCC_compiler.try_flags(["-lblas"])
+    """
+    TODO FIXME: This is a very poor test.
+
+    Check that the code detecting blas flags does not raise any exception.
+    It used to happen on Python 3 because of improper string handling,
+    but was not detected because that path is not usually taken,
+    so we test it here directly.
+    """
+    res = GCC_compiler.try_flags(["-lblas"])
+    assert isinstance(res, bool)
 
 
 @patch("aesara.link.c.cmodule.try_blas_flag", return_value=None)

tests/link/c/test_type.py

@@ -11,66 +11,68 @@ from aesara.link.c.type import CDataType, CEnumType, EnumList, EnumType
 from aesara.tensor.type import TensorType, continuous_dtypes
 
 
-@pytest.mark.skipif(
-    not aesara.config.cxx, reason="G++ not available, so we need to skip this test."
-)
-def test_cdata():
-    class ProdOp(COp):
-        __props__ = ()
-
-        def make_node(self, i):
-            return Apply(self, [i], [CDataType("void *", "py_decref")()])
-
-        def c_support_code(self, **kwargs):
-            return """
-    void py_decref(void *p) {
-    Py_XDECREF((PyObject *)p);
-    }
-    """
-
-        def c_code(self, node, name, inps, outs, sub):
-            return """
-    Py_XDECREF(%(out)s);
-    %(out)s = (void *)%(inp)s;
-    Py_INCREF(%(inp)s);
-    """ % dict(
-                out=outs[0], inp=inps[0]
-            )
+class ProdOp(COp):
+    __props__ = ()
+
+    def make_node(self, i):
+        return Apply(self, [i], [CDataType("void *", "py_decref")()])
 
-        def c_code_cache_version(self):
-            return (0,)
+    def c_support_code(self, **kwargs):
+        return """
+void py_decref(void *p) {
+Py_XDECREF((PyObject *)p);
+}
+"""
 
-        def perform(self, *args, **kwargs):
-            raise NotImplementedError()
+    def c_code(self, node, name, inps, outs, sub):
+        return """
+Py_XDECREF(%(out)s);
+%(out)s = (void *)%(inp)s;
+Py_INCREF(%(inp)s);
+""" % dict(
+            out=outs[0], inp=inps[0]
+        )
 
-    class GetOp(COp):
-        __props__ = ()
+    def c_code_cache_version(self):
+        return (0,)
 
-        def make_node(self, c):
-            return Apply(self, [c], [TensorType("float32", (False,))()])
+    def perform(self, *args, **kwargs):
+        raise NotImplementedError()
 
-        def c_support_code(self, **kwargs):
-            return """
-    void py_decref(void *p) {
-    Py_XDECREF((PyObject *)p);
-    }
-    """
 
-        def c_code(self, node, name, inps, outs, sub):
-            return """
-    Py_XDECREF(%(out)s);
-    %(out)s = (PyArrayObject *)%(inp)s;
-    Py_INCREF(%(out)s);
-    """ % dict(
-                out=outs[0], inp=inps[0]
-            )
+class GetOp(COp):
+    __props__ = ()
 
-        def c_code_cache_version(self):
-            return (0,)
+    def make_node(self, c):
+        return Apply(self, [c], [TensorType("float32", (False,))()])
 
-        def perform(self, *args, **kwargs):
-            raise NotImplementedError()
+    def c_support_code(self, **kwargs):
+        return """
+void py_decref(void *p) {
+Py_XDECREF((PyObject *)p);
+}
+"""
 
+    def c_code(self, node, name, inps, outs, sub):
+        return """
+Py_XDECREF(%(out)s);
+%(out)s = (PyArrayObject *)%(inp)s;
+Py_INCREF(%(out)s);
+""" % dict(
+            out=outs[0], inp=inps[0]
+        )
+
+    def c_code_cache_version(self):
+        return (0,)
+
+    def perform(self, *args, **kwargs):
+        raise NotImplementedError()
+
+
+@pytest.mark.skipif(
+    not aesara.config.cxx, reason="G++ not available, so we need to skip this test."
+)
+def test_cdata():
     i = TensorType("float32", (False,))()
     c = ProdOp()(i)
     i2 = GetOp()(c)