Node: add the is_valid_cache setter property (#5207)

This property allows to mark an individual node as invalid for use as a
caching source. It works by setting the `_aiida_valid_cache` extra to
either `True` or `False`.

Previously, the unofficial way of ensuring a node was no longer used as
a cache source, was to remove the `_aiida_hash` extra, containing the
hash of the node. This would achieve the desired effect, since without a
hash, the node would never be found when looking for identical nodes to
cache from. However, this not an ideal approach because the hash may be
useful for other purposes. In addition, if the node were to be rehashed
at some point, which could happen by accident if all nodes of a
particular class got rehashed, it would all of a sudden be used in
caching again.

The solution is to have a way to persistently mark node instances as
invalid for caching. Note that the `is_valid_cache` property of the
`Node` before was intended as a hook to allow changing the caching
behavior of subclasses of `Node` as a whole, and not on the instance
level. This is now changed as the property has a setter that operates on
the instance level. Subclasses should take care to respect this base
class implementation if it makes sense. Since `Node` can only be
subclassed by `aiida-core` and not in plugins, we can easily enforce
this system.

aiida/common/hashing.py

@@ -41,9 +41,6 @@
 
 HASHING_KEY = 'HashingKey'
 
-# The key that is used to store the hash in the node extras
-_HASH_EXTRA_KEY = '_aiida_hash'
-
 ###################################################################
 # THE FOLLOWING WAS TAKEN FROM DJANGO BUT IT CAN BE EASILY REPLACED
 ###################################################################

aiida/orm/nodes/node.py

@@ -32,7 +32,7 @@
 
 from aiida.common import exceptions
 from aiida.common.escaping import sql_string_match
-from aiida.common.hashing import _HASH_EXTRA_KEY, make_hash
+from aiida.common.hashing import make_hash
 from aiida.common.lang import classproperty, type_check
 from aiida.common.links import LinkType
 from aiida.manage.manager import get_manager
@@ -122,6 +122,10 @@ class Node(
     """
     # pylint: disable=too-many-public-methods
 
+    # The keys in the extras that are used to store the hash of the node and whether it should be used in caching.
+    _HASH_EXTRA_KEY: str = '_aiida_hash'
+    _VALID_CACHE_KEY: str = '_aiida_valid_cache'
+
     # added by metaclass
     _plugin_type_string: ClassVar[str]
     _query_type_string: ClassVar[str]
@@ -742,7 +746,7 @@ def _store(self, with_transaction: bool = True, clean: bool = True) -> 'Node':
         self._backend_entity.store(links, with_transaction=with_transaction, clean=clean)
 
         self._incoming_cache = []
-        self._backend_entity.set_extra(_HASH_EXTRA_KEY, self.get_hash())
+        self._backend_entity.set_extra(self._HASH_EXTRA_KEY, self.get_hash())
 
         return self
 
@@ -848,11 +852,11 @@ def _get_objects_to_hash(self) -> List[Any]:
 
     def rehash(self) -> None:
         """Regenerate the stored hash of the Node."""
-        self.set_extra(_HASH_EXTRA_KEY, self.get_hash())
+        self.set_extra(self._HASH_EXTRA_KEY, self.get_hash())
 
     def clear_hash(self) -> None:
         """Sets the stored hash of the Node to None."""
-        self.set_extra(_HASH_EXTRA_KEY, None)
+        self.set_extra(self._HASH_EXTRA_KEY, None)
 
     def get_cache_source(self) -> Optional[str]:
         """Return the UUID of the node that was used in creating this node from the cache, or None if it was not cached.
@@ -905,22 +909,38 @@ def _iter_all_same_nodes(self, allow_before_store=False) -> Iterator['Node']:
         """
         if not allow_before_store and not self.is_stored:
             raise exceptions.InvalidOperation('You can get the hash only after having stored the node')
+
         node_hash = self._get_hash()
 
         if not node_hash or not self._cachable:
             return iter(())
 
         builder = QueryBuilder(backend=self.backend)
-        builder.append(self.__class__, filters={'extras._aiida_hash': node_hash}, project='*', subclassing=False)
-        nodes_identical = (n[0] for n in builder.iterall())
+        builder.append(self.__class__, filters={f'extras.{self._HASH_EXTRA_KEY}': node_hash}, subclassing=False)
 
-        return (node for node in nodes_identical if node.is_valid_cache)
+        return (node for node in builder.all(flat=True) if node.is_valid_cache)  # type: ignore[misc,union-attr]
 
     @property
     def is_valid_cache(self) -> bool:
-        """Hook to exclude certain `Node` instances from being considered a valid cache."""
-        # pylint: disable=no-self-use
-        return True
+        """Hook to exclude certain ``Node`` classes from being considered a valid cache.
+
+        The base class assumes that all node instances are valid to cache from, unless the ``_VALID_CACHE_KEY`` extra
+        has been set to ``False`` explicitly. Subclasses can override this property with more specific logic, but should
+        probably also consider the value returned by this base class.
+        """
+        return self.get_extra(self._VALID_CACHE_KEY, True)
+
+    @is_valid_cache.setter
+    def is_valid_cache(self, valid: bool) -> None:
+        """Set whether this node instance is considered valid for caching or not.
+
+        If a node instance has this property set to ``False``, it will never be used in the caching mechanism, unless
+        the subclass overrides the ``is_valid_cache`` property and ignores it implementation completely.
+
+        :param valid: whether the node is valid or invalid for use in caching.
+        """
+        type_check(valid, bool)
+        self.set_extra(self._VALID_CACHE_KEY, valid)
 
     def get_description(self) -> str:
         """Return a string with a description of the node.

aiida/orm/nodes/process/process.py

@@ -496,6 +496,14 @@ def is_valid_cache(self) -> bool:
 
         return is_valid_cache_func(self)
 
+    @is_valid_cache.setter
+    def is_valid_cache(self, valid: bool) -> None:
+        """Set whether this node instance is considered valid for caching or not.
+
+        :param valid: whether the node is valid or invalid for use in caching.
+        """
+        super().is_valid_cache = valid  # type: ignore[misc]
+
     def _get_objects_to_hash(self) -> List[Any]:
         """
         Return a list of objects which should be included in the hash.

docs/source/howto/run_codes.rst

@@ -497,7 +497,7 @@ Besides the on/off switch set by ``caching.default_enabled``, caching can be con
     caching.default_enabled  profile   True
     caching.disabled_for     profile   aiida.calculations:core.templatereplacer
     caching.enabled_for      profile   aiida.calculations:quantumespresso.pw
-                                    aiida.calculations:other
+                                       aiida.calculations:other
 
 In this example, caching is enabled by default, but explicitly disabled for calculations of the ``TemplatereplacerCalculation`` class, identified by its corresponding ``aiida.calculations:core.templatereplacer`` entry point string.
 It also shows how to enable caching for particular calculations (which has no effect here due to the profile-wide default).
@@ -573,22 +573,9 @@ Caching can be enabled or disabled on a case-by-case basis by using the :class:`
     This affects only the current Python interpreter and won't change the behavior of the daemon workers.
     This means that this technique is only useful when using :py:class:`~aiida.engine.run`, and **not** with :py:class:`~aiida.engine.submit`.
 
-If you suspect a node is being reused in error (e.g. during development), you can also manually *prevent* a specific node from being reused:
-
-#. Load one of the nodes you suspect to be a clone.
-   Check that :meth:`~aiida.orm.nodes.Node.get_cache_source` returns a UUID.
-   If it returns `None`, the node was not cloned.
-
-#. Clear the hashes of all nodes that are considered identical to this node:
-
-    .. code:: python
-
-        for node in node.get_all_same_nodes():
-            node.clear_hash()
-
-#. Run your calculation again.
-   The node in question should no longer be reused.
 
+Besides controlling which process classes are cached, it may be useful or necessary to control what already _stored_ nodes are used as caching _sources_.
+Section :ref:`topics:provenance:caching:control-caching` provides details how AiiDA decides which stored nodes are equivalent to the node being stored and which are considered valid caching sources.
 
 .. |Computer| replace:: :py:class:`~aiida.orm.Computer`
 .. |CalcJob| replace:: :py:class:`~aiida.engine.processes.calcjobs.calcjob.CalcJob`

docs/source/topics/provenance/caching.rst

@@ -87,13 +87,58 @@ For implementation details of the hashing mechanism for process nodes, see :ref:
 Controlling Caching
 -------------------
 
-Caching can be configured at runtime (see :ref:`how-to:run-codes:caching:configure`) and when implementing a new process class:
+In the caching mechanism, there are two different types of roles played by the nodes: the node that is currently being stored is called the `target`, and the nodes already stored in the database that are considered to be equivalent are referred to as a `source`.
+
+Targets
+.......
+
+Controlling what nodes will look in the database for existing equivalents when being stored is done on the class level.
+Section :ref:`how-to:run-codes:caching:configure` explains how this can be controlled globally through the profile configuration, or locally through context managers.
+
+Sources
+.......
+
+When a node is being stored (the `target`) and caching is enabled for its node class (see section above), a valid cache `source` is obtained through the method :meth:`~aiida.orm.nodes.node.Node._get_same_node`.
+This method calls the iterator :meth:`~aiida.orm.nodes.node.Node._iter_all_same_nodes` and takes the first one it returns if there are any.
+To find the list of `source` nodes that are equivalent to the `target` that is being stored, :meth:`~aiida.orm.nodes.node.Node._iter_all_same_nodes` performs the following steps:
+
+ 1. It queries the database for all nodes that have the same hash as the `target` node.
+ 2. From the result, only those nodes are returned where the property :meth:`~aiida.orm.nodes.node.Node.is_valid_cache` returns ``True``.
+
+The property :meth:`~aiida.orm.nodes.node.Node.is_valid_cache` therefore allows to control whether a stored node can be used as a `source` in the caching mechanism.
+By default, for all nodes, the property returns ``True``.
+However, this can be changed on a per-node basis, by setting it to ``False``
+
+.. code-block:: python
+
+    node = load_node(<IDENTIFIER>)
+    node.is_valid_cache = False
+
+Setting the property to ``False``, will cause an extra to be stored on the node in the database, such that even when it is loaded at a later point in time, ``is_valid_cache`` returns ``False``.
+
+.. code-block:: python
+
+    node = load_node(<IDENTIFIER>)
+    assert node.is_valid_cache is False
+
+Through this method, it is possible to guarantee that individual nodes are never used as a `source` for caching.
+
+The :class:`~aiida.engine.processes.process.Process` class overrides the :meth:`~aiida.orm.nodes.node.Node.is_valid_cache` property to give more fine-grained control on process nodes as caching sources.
+If either :meth:`~aiida.orm.nodes.node.Node.is_valid_cache` of the base class or :meth:`~aiida.orm.nodes.process.process.ProcessNode.is_finished` returns ``False``, the process node is not a valid source.
+Likewise, if the process class cannot be loaded from the node, through the :meth:`~aiida.orm.nodes.process.process.ProcessNode.process_class`, the node is not a valid caching source.
+Finally, if the associated process class implements the :meth:`~aiida.engine.processes.process.Process.is_valid_cache` method, it is called, passing the node as an argument.
+If that returns ``True``, the node is considered to be a valid caching source.
+
+The :meth:`~aiida.engine.processes.process.Process.is_valid_cache` is implemented on the :class:`~aiida.engine.processes.process.Process` class.
+It will check whether the exit code that is set on the node, if any, has the keyword argument ``invalidates_cache`` set to ``True``, in which case the property will return ``False`` indicating the node is not a valid caching source.
+Whether an exit code invalidates the cache, is controlled with the ``invalidates_cache`` argument when it is defined on the process spec through the :meth:`spec.exit_code <aiida.engine.processes.process_spec.ProcessSpec.exit_code>` method.
+
+.. warning::
+
+    Process plugins can override the :meth:`~aiida.engine.processes.process.Process.is_valid_cache` method, to further control how nodes are considered valid caching sources.
+    When doing so, make sure to call :meth:`super().is_valid_cache(node) <aiida.engine.processes.process.Process.is_valid_cache>` and respect its output: if it is `False`, your implementation should also return `False`.
+    If you do not comply with this, the ``invalidates_cache`` keyword on exit codes will no longer work.
 
-* The :meth:`spec.exit_code <aiida.engine.processes.process_spec.ProcessSpec.exit_code>` has a keyword argument ``invalidates_cache``.
-  If this is set to ``True``, that means that a calculation with this exit code will not be used as a cache source for another one, even if their hashes match.
-* The :class:`Process <aiida.engine.processes.process.Process>` parent class from which calcjobs inherit has an :meth:`is_valid_cache <aiida.engine.processes.process.Process.is_valid_cache>` method, which can be overridden in the plugin to implement custom ways of invalidating the cache.
-  When doing this, make sure to call :meth:`super().is_valid_cache(node)<aiida.engine.processes.process.Process.is_valid_cache>` and respect its output: if it is `False`, your implementation should also return `False`.
-  If you do not comply with this, the 'invalidates_cache' keyword on exit codes will not work.
 
 .. _topics:provenance:caching:limitations:
 

tests/orm/node/test_node.py

@@ -917,56 +917,66 @@ def test_remove_comment(self):
 
 
 @pytest.mark.usefixtures('clear_database_before_test')
-def test_store_from_cache():
-    """Regression test for storing a Node with (nested) repository content with caching."""
-    data = Data()
-    with tempfile.TemporaryDirectory() as tmpdir:
-        dir_path = os.path.join(tmpdir, 'directory')
-        os.makedirs(dir_path)
-        with open(os.path.join(dir_path, 'file'), 'w', encoding='utf8') as file:
-            file.write('content')
-        data.put_object_from_tree(tmpdir)
+class TestNodeCaching:
+    """Tests the caching behavior of the ``Node`` class."""
 
-    data.store()
+    def test_is_valid_cache(self):
+        """Test the ``Node.is_valid_cache`` property."""
+        node = Node()
+        assert node.is_valid_cache
 
-    clone = data.clone()
-    clone._store_from_cache(data, with_transaction=True)  # pylint: disable=protected-access
+        node.is_valid_cache = False
+        assert not node.is_valid_cache
 
-    assert clone.is_stored
-    assert clone.get_cache_source() == data.uuid
-    assert data.get_hash() == clone.get_hash()
-
-
-@pytest.mark.usefixtures('clear_database_before_test')
-def test_hashing_errors(aiida_caplog):
-    """Tests that ``get_hash`` fails in an expected manner."""
-    node = Data().store()
-    node.__module__ = 'unknown'  # this will inhibit package version determination
-    result = node.get_hash(ignore_errors=True)
-    assert result is None
-    assert aiida_caplog.record_tuples == [(node.logger.name, logging.ERROR, 'Node hashing failed')]
-
-    with pytest.raises(exceptions.HashingError, match='package version could not be determined'):
-        result = node.get_hash(ignore_errors=False)
-    assert result is None
+        with pytest.raises(TypeError):
+            node.is_valid_cache = 'false'
 
+    def test_store_from_cache(self):
+        """Regression test for storing a Node with (nested) repository content with caching."""
+        data = Data()
+        with tempfile.TemporaryDirectory() as tmpdir:
+            dir_path = os.path.join(tmpdir, 'directory')
+            os.makedirs(dir_path)
+            with open(os.path.join(dir_path, 'file'), 'w', encoding='utf8') as file:
+                file.write('content')
+            data.put_object_from_tree(tmpdir)
 
-@pytest.mark.usefixtures('clear_database_before_test')
-def test_uuid_equality_fallback():
-    """Tests the fallback mechanism of checking equality by comparing uuids and hash."""
-    node_0 = Data().store()
+        data.store()
 
-    nodepk = Data().store().pk
-    node_a = load_node(pk=nodepk)
-    node_b = load_node(pk=nodepk)
+        clone = data.clone()
+        clone._store_from_cache(data, with_transaction=True)  # pylint: disable=protected-access
 
-    assert node_a == node_b
-    assert node_a != node_0
-    assert node_b != node_0
+        assert clone.is_stored
+        assert clone.get_cache_source() == data.uuid
+        assert data.get_hash() == clone.get_hash()
 
-    assert hash(node_a) == hash(node_b)
-    assert hash(node_a) != hash(node_0)
-    assert hash(node_b) != hash(node_0)
+    def test_hashing_errors(self, aiida_caplog):
+        """Tests that ``get_hash`` fails in an expected manner."""
+        node = Data().store()
+        node.__module__ = 'unknown'  # this will inhibit package version determination
+        result = node.get_hash(ignore_errors=True)
+        assert result is None
+        assert aiida_caplog.record_tuples == [(node.logger.name, logging.ERROR, 'Node hashing failed')]
+
+        with pytest.raises(exceptions.HashingError, match='package version could not be determined'):
+            result = node.get_hash(ignore_errors=False)
+        assert result is None
+
+    def test_uuid_equality_fallback(self):
+        """Tests the fallback mechanism of checking equality by comparing uuids and hash."""
+        node_0 = Data().store()
+
+        nodepk = Data().store().pk
+        node_a = load_node(pk=nodepk)
+        node_b = load_node(pk=nodepk)
+
+        assert node_a == node_b
+        assert node_a != node_0
+        assert node_b != node_0
+
+        assert hash(node_a) == hash(node_b)
+        assert hash(node_a) != hash(node_0)
+        assert hash(node_b) != hash(node_0)
 
 
 @pytest.mark.usefixtures('clear_database_before_test')