🐛`RecursionError` when trying to `copy()` a computer.

[mbercx]

trafficstars

Describe the bug

When trying to use the copy() command on a computer, I get a RecursionError:

In [1]: load_computer('localhost').copy()
---------------------------------------------------------------------------
RecursionError                            Traceback (most recent call last)
Cell In[1], line 1
----> 1 load_computer('localhost').copy()

File ~/project/core/code/aiida-core/aiida/orm/computers.py:279, in Computer.copy(self)
    275 def copy(self) -> 'Computer':
    276     """
    277     Return a copy of the current object to work with, not stored yet.
    278     """
--> 279     return entities.from_backend_entity(Computer, self._backend_entity.copy())

File ~/project/core/code/aiida-core/aiida/storage/psql_dos/orm/computers.py:59, in SqlaComputer.copy(self)
     56 if not self.is_stored:
     57     raise exceptions.InvalidOperation('You can copy a computer only after having stored it')
---> 59 dbcomputer = copy(self.model)
     60 make_transient(dbcomputer)
     61 session.add(dbcomputer)

File /opt/homebrew/Cellar/[email protected]/3.9.18/Frameworks/Python.framework/Versions/3.9/lib/python3.9/copy.py:102, in copy(x)
    100 if isinstance(rv, str):
    101     return x
--> 102 return _reconstruct(x, None, *rv)

File /opt/homebrew/Cellar/[email protected]/3.9.18/Frameworks/Python.framework/Versions/3.9/lib/python3.9/copy.py:271, in _reconstruct(x, memo, func, args, state, listiter, dictiter, deepcopy)
    269 if deep:
    270     state = deepcopy(state, memo)
--> 271 if hasattr(y, '__setstate__'):
    272     y.__setstate__(state)
    273 else:

File ~/project/core/code/aiida-core/aiida/storage/psql_dos/orm/utils.py:84, in ModelWrapper.__getattr__(self, item)
     81 if item in ('_model', '_backend'):
     82     raise AttributeError()
---> 84 if self.is_saved() and self._is_mutable_model_field(item) and not self._in_transaction():
     85     self._ensure_model_uptodate(fields=(item,))
     87 return getattr(self._model, item)

File ~/project/core/code/aiida-core/aiida/storage/psql_dos/orm/utils.py:109, in ModelWrapper.is_saved(self)
    103 """Return whether the wrapped model instance is saved in the database.
    104 
    105 :return: boolean, True if the model is saved in the database, False otherwise
    106 """
    107 # we should not flush here since it may lead to IntegrityErrors
    108 # which are handled later in the save method
--> 109 with self.session.no_autoflush:
    110     return self._model.id is not None

File ~/project/core/code/aiida-core/aiida/storage/psql_dos/orm/utils.py:84, in ModelWrapper.__getattr__(self, item)
     81 if item in ('_model', '_backend'):
     82     raise AttributeError()
---> 84 if self.is_saved() and self._is_mutable_model_field(item) and not self._in_transaction():
     85     self._ensure_model_uptodate(fields=(item,))
     87 return getattr(self._model, item)

File ~/project/core/code/aiida-core/aiida/storage/psql_dos/orm/utils.py:109, in ModelWrapper.is_saved(self)
    103 """Return whether the wrapped model instance is saved in the database.
    104 
    105 :return: boolean, True if the model is saved in the database, False otherwise
    106 """
    107 # we should not flush here since it may lead to IntegrityErrors
    108 # which are handled later in the save method
--> 109 with self.session.no_autoflush:
    110     return self._model.id is not None

    [... skipping similar frames: ModelWrapper.__getattr__ at line 84 (1480 times), ModelWrapper.is_saved at line 109 (1480 times)]

File ~/project/core/code/aiida-core/aiida/storage/psql_dos/orm/utils.py:84, in ModelWrapper.__getattr__(self, item)
     81 if item in ('_model', '_backend'):
     82     raise AttributeError()
---> 84 if self.is_saved() and self._is_mutable_model_field(item) and not self._in_transaction():
     85     self._ensure_model_uptodate(fields=(item,))
     87 return getattr(self._model, item)

File ~/project/core/code/aiida-core/aiida/storage/psql_dos/orm/utils.py:109, in ModelWrapper.is_saved(self)
    103 """Return whether the wrapped model instance is saved in the database.
    104 
    105 :return: boolean, True if the model is saved in the database, False otherwise
    106 """
    107 # we should not flush here since it may lead to IntegrityErrors
    108 # which are handled later in the save method
--> 109 with self.session.no_autoflush:
    110     return self._model.id is not None

File ~/project/core/code/aiida-core/aiida/storage/psql_dos/orm/utils.py:67, in ModelWrapper.session(self)
     64 @property
     65 def session(self) -> Session:
     66     """Return the session of the storage backend instance."""
---> 67     return self._backend.get_session()

File ~/project/core/code/aiida-core/aiida/storage/psql_dos/orm/utils.py:81, in ModelWrapper.__getattr__(self, item)
     70 """Get an attribute of the model instance.
     71 
     72 If the model is saved in the database, the item corresponds to a mutable model field and the current scope is
   (...)
     76 :return: the value of the model's attribute
     77 """
     78 # Python 3's implementation of copy.copy does not call __init__ on the new object
     79 # but manually restores attributes instead. Make sure we never get into a recursive
     80 # loop by protecting the special variables here
---> 81 if item in ('_model', '_backend'):
     82     raise AttributeError()
     84 if self.is_saved() and self._is_mutable_model_field(item) and not self._in_transaction():

RecursionError: maximum recursion depth exceeded in comparison

Steps to reproduce

Steps to reproduce the behavior:

1. Load any computer.
2. Try to copy it.

Your environment

• Operating system [e.g. Linux]: macOS Ventura 13.5.1
• Python version [e.g. 3.7.1]: 3.9.18
• aiida-core version [e.g. 1.2.1]: main branch.

[sphuber]

What is the goal here? To get a clone of the computer that is unstored? We had the same problem for Node. The ModelWrapper is causing issues because it is overloading the __getattr__ method. For the Node we decided that the concept of copying is complicated because due to the various uniqueness constraints and the concept of storing. If the node is stored, should the clone also be stored? But that would violate the constraints. Of course you could change the PK/UUID, but then it wouldn't be strictly a copy anymore.

In the end, we decided to disable the copy method and only allow deepcopy and essentially have that forward to the clone method, which returns an unstored clone of the node:

def __copy__(self):
    """Copying a Data node is not supported, use copy.deepcopy or call Data.clone()."""
    raise exceptions.InvalidOperation('copying a Data node is not supported, use copy.deepcopy')

def __deepcopy__(self, memo):
    """
    Create a clone of the Data node by piping through to the clone method and return the result.

    :returns: an unstored clone of this Data node
    """
    return self.clone()

We could move this logic up to the Entity such that it is applied to all database entities. What do you think?

[mbercx]

Without having considered all the implications, this would work for me. For context, I was trying to on the fly create a computer which is exactly the same as the one I'm running my code on, but using the core.direct scheduler instead of core.slurm. I'll explain the use case on Discourse at some point, don't want to derail the issue here.

[sphuber]

For that purpose there is of course the verdi computer duplicate command, but might be nice to have an easy way in the API