Safeguards for ModelMember.parent and ModelMember.is_equivalent (#657)

This PR resolves #600 and #651.

Author: rileyjmurray

pygsti/modelmembers/modelmember.py

@@ -254,6 +254,10 @@ def parent(self):
         -------
         Model
         """
+        if '_parent' not in self.__dict__:
+            # This can be absent because of how serialization works.
+            # It's set during deserialization in self.relink_parent().
+            self._parent = None
         return self._parent
 
     @parent.setter
@@ -308,11 +312,13 @@ def relink_parent(self, parent):
         None
         """
         for subm in self.submembers():
-            subm.relink_parent(parent)
-
-        if self._parent is parent: return  # OK to relink multiple times
-        assert(self._parent is None), "Cannot relink parent: parent is not None!"
-        self._parent = parent  # assume no dependent objects
+            subm.relink_parent(parent)    
+        if '_parent' in self.__dict__:
+            # This codepath needed to resolve GitHub issue 651.
+            if self._parent is parent:
+                return  # OK to relink multiple times
+            assert(self._parent is None), "Cannot relink parent: current parent is not None!"
+        self._parent = parent
 
     def unlink_parent(self, force=False):
         """
@@ -793,6 +799,9 @@ def copy(self, parent=None, memo=None):
             memo[id(self.parent)] = None  # so deepcopy uses None instead of copying parent
         return self._copy_gpindices(_copy.deepcopy(self, memo), parent, memo)
 
+    def to_dense(self) -> _np.ndarray:
+        raise NotImplementedError('Derived classes must implement .to_dense().')
+
     def _is_similar(self, other, rtol, atol):
         """ Returns True if `other` model member (which it guaranteed to be the same type as self) has
             the same local structure, i.e., not considering parameter values or submembers """
@@ -857,7 +866,16 @@ def is_equivalent(self, other, rtol=1e-5, atol=1e-8):
         """
         if not self.is_similar(other): return False
 
-        if not _np.allclose(self.to_vector(), other.to_vector(), rtol=rtol, atol=atol):
+        try:
+            v1 = self.to_vector()
+            v2 = other.to_vector()
+        except RuntimeError as e:
+            if 'to_vector() should never be called' not in str(e):
+                raise e
+            assert type(self) == type(other)
+            v1 = self.to_dense()
+            v2 = other.to_dense()
+        if not _np.allclose(v1, v2, rtol=rtol, atol=atol):
             return False
 
         # Recursive check on submembers

test/unit/modelmembers/test_operation.py

@@ -394,6 +394,26 @@ def test_convert(self):
         conv = op.convert(self.gate, "full TP", "gm")
         # TODO assert correctness
 
+    def test_deepcopy(self):
+        # Issue #651 showed that an infinite recursion error is possible
+        # when deep-copying a FullTPOp whose ._parent field is not None.
+        # This test triggers that code path. It should pass after applying
+        # the proposed fix in #651.
+        #
+        # In order to use m1.is_equivalent(m2) in the correctness check
+        # we had to resolve GitHub issue #600.
+        import copy
+        from pygsti.modelpacks import smq1Q_XY
+        m1  = smq1Q_XY.target_model('full TP')
+        o1 = m1.operations[('Gxpi2',0)]
+        o2 = copy.deepcopy(o1)
+        m2 = o2.parent
+        assert id(o1) != id(o2)
+        assert id(m1) != id(m2)
+        res = m1.is_equivalent(m2, rtol=0, atol=0)
+        assert res
+        return
+
     def test_first_row_read_only(self):
         # check that first row is read-only
         e1 = self.gate[0, 0]