Deprecate qid_pairs (#4900)

Deprecate `qid_pairs`. Next step on #4744 .

This also makes a small update to the DeviceMetadata to make optional fields required (since the metadata property itself is optional this seems fine).

cirq-core/cirq/contrib/graph_device/graph_device.py

@@ -17,7 +17,7 @@
 
 from typing import Iterable, Optional, FrozenSet, TYPE_CHECKING, Tuple, cast
 
-from cirq import devices, ops, value
+from cirq import _compat, devices, ops, value
 
 from cirq.contrib.graph_device.hypergraph import UndirectedHypergraph
 
@@ -162,14 +162,19 @@ def qubit_set(self) -> FrozenSet['cirq.Qid']:
     def edges(self):
         return tuple(sorted(self.device_graph.edges))
 
+    @_compat.deprecated(
+        deadline='v0.15',
+        fix='qubit coupling data can now be found in device.metadata.nx_graph if provided.',
+    )
     def qid_pairs(self) -> FrozenSet['cirq.SymmetricalQidPair']:
-        return frozenset(
-            [
-                devices.SymmetricalQidPair(*edge)  # type: ignore
-                for edge in self.device_graph.edges
-                if len(edge) == 2 and all(isinstance(q, ops.Qid) for q in edge)
-            ]
-        )
+        with _compat.block_overlapping_deprecation('device\\.metadata'):
+            return frozenset(
+                [
+                    devices.SymmetricalQidPair(*edge)  # type: ignore
+                    for edge in self.device_graph.edges
+                    if len(edge) == 2 and all(isinstance(q, ops.Qid) for q in edge)
+                ]
+            )
 
     @property
     def labelled_edges(self):

cirq-core/cirq/contrib/graph_device/graph_device_test.py

@@ -182,8 +182,9 @@ def test_qubit_set():
     assert device.qubit_set() == {a, b, c, d}
 
 
-def test_qid_pairs():
+def test_qid_pairs_deprecated():
     a, b, c, d = cirq.LineQubit.range(4)
     device_graph = ccgd.UndirectedHypergraph(labelled_edges={(a, b): None, (c, d): None})
     device = ccgd.UndirectedGraphDevice(device_graph=device_graph)
-    assert len(device.qid_pairs()) == 2
+    with cirq.testing.assert_deprecated('device.metadata', deadline='v0.15', count=1):
+        assert len(device.qid_pairs()) == 2

cirq-core/cirq/devices/device.py

@@ -22,9 +22,8 @@
     Iterator,
     Iterable,
 )
-
 import networkx as nx
-from cirq import value
+from cirq import _compat, value
 from cirq.devices.grid_qubit import _BaseGridQid
 from cirq.devices.line_qubit import _BaseLineQid
 
@@ -59,6 +58,10 @@ def qubit_set(self) -> Optional[AbstractSet['cirq.Qid']]:
         # Default to the qubits being unknown.
         return None
 
+    @_compat.deprecated(
+        deadline='v0.15',
+        fix='qubit coupling data can now be found in device.metadata if provided.',
+    )
     def qid_pairs(self) -> Optional[FrozenSet['cirq.SymmetricalQidPair']]:
         """Returns a set of qubit edges on the device, if possible.
 
@@ -74,27 +77,28 @@ def qid_pairs(self) -> Optional[FrozenSet['cirq.SymmetricalQidPair']]:
             `cirq.UnconstrainedDevice` has this property), then `None` is
             returned.
         """
-        qs = self.qubit_set()
-        if qs is None:
-            return None
-        if all(isinstance(q, _BaseGridQid) for q in qs):
-            return frozenset(
-                [
-                    SymmetricalQidPair(q, q2)
-                    for q in [cast(_BaseGridQid, q) for q in qs]
-                    for q2 in [q + (0, 1), q + (1, 0)]
-                    if q2 in qs
-                ]
-            )
-        if all(isinstance(q, _BaseLineQid) for q in qs):
-            return frozenset(
-                [
-                    SymmetricalQidPair(q, q + 1)
-                    for q in [cast(_BaseLineQid, q) for q in qs]
-                    if q + 1 in qs
-                ]
-            )
-        return frozenset([SymmetricalQidPair(q, q2) for q in qs for q2 in qs if q < q2])
+        with _compat.block_overlapping_deprecation('device\\.metadata'):
+            qs = self.qubit_set()
+            if qs is None:
+                return None
+            if all(isinstance(q, _BaseGridQid) for q in qs):
+                return frozenset(
+                    [
+                        SymmetricalQidPair(q, q2)
+                        for q in [cast(_BaseGridQid, q) for q in qs]
+                        for q2 in [q + (0, 1), q + (1, 0)]
+                        if q2 in qs
+                    ]
+                )
+            if all(isinstance(q, _BaseLineQid) for q in qs):
+                return frozenset(
+                    [
+                        SymmetricalQidPair(q, q + 1)
+                        for q in [cast(_BaseLineQid, q) for q in qs]
+                        if q + 1 in qs
+                    ]
+                )
+            return frozenset([SymmetricalQidPair(q, q2) for q in qs for q2 in qs if q < q2])
 
     def decompose_operation(self, operation: 'cirq.Operation') -> 'cirq.OP_TREE':
         """Returns a device-valid decomposition for the given operation.
@@ -166,6 +170,10 @@ def can_add_operation_into_moment(
         return not moment.operates_on(operation.qubits)
 
 
+@_compat.deprecated_class(
+    deadline='v0.15',
+    fix='Qid coupling information can now be found in device.metadata if applicable.',
+)
 @value.value_equality
 class SymmetricalQidPair:
     def __init__(self, qid1: 'cirq.Qid', qid2: 'cirq.Qid'):
@@ -204,8 +212,8 @@ class DeviceMetadata:
 
     def __init__(
         self,
-        qubits: Optional[Iterable['cirq.Qid']] = None,
-        nx_graph: Optional['nx.graph'] = None,
+        qubits: Iterable['cirq.Qid'],
+        nx_graph: 'nx.graph',
     ):
         """Construct a DeviceMetadata object.
 
@@ -216,16 +224,11 @@ def __init__(
                 directional coupling, undirected edges indicate bi-directional
                 coupling.
         """
-        if qubits is not None:
-            qubits = frozenset(qubits)
-        self._qubits_set: Optional[FrozenSet['cirq.Qid']] = (
-            None if qubits is None else frozenset(qubits)
-        )
-
+        self._qubits_set: FrozenSet['cirq.Qid'] = frozenset(qubits)
         self._nx_graph = nx_graph
 
     @property
-    def qubit_set(self) -> Optional[FrozenSet['cirq.Qid']]:
+    def qubit_set(self) -> FrozenSet['cirq.Qid']:
         """Returns a set of qubits on the device, if possible.
 
         Returns:
@@ -234,7 +237,7 @@ def qubit_set(self) -> Optional[FrozenSet['cirq.Qid']]:
         return self._qubits_set
 
     @property
-    def nx_graph(self) -> Optional['nx.Graph']:
+    def nx_graph(self) -> 'nx.Graph':
         """Returns a nx.Graph where nodes are qubits and edges are couple-able qubits.
 
         Returns:
@@ -243,31 +246,20 @@ def nx_graph(self) -> Optional['nx.Graph']:
         return self._nx_graph
 
     def _value_equality_values_(self):
-        graph_equality = None
-        if self._nx_graph is not None:
-            graph_equality = (
-                tuple(sorted(self._nx_graph.nodes())),
-                tuple(sorted(self._nx_graph.edges(data='directed'))),
-            )
+        graph_equality = (
+            tuple(sorted(self._nx_graph.nodes())),
+            tuple(sorted(self._nx_graph.edges(data='directed'))),
+        )
 
         return self._qubits_set, graph_equality
 
     def _json_dict_(self):
-        graph_payload = ''
-        if self._nx_graph is not None:
-            graph_payload = nx.readwrite.json_graph.node_link_data(self._nx_graph)
-
-        qubits_payload = ''
-        if self._qubits_set is not None:
-            qubits_payload = sorted(list(self._qubits_set))
+        graph_payload = nx.readwrite.json_graph.node_link_data(self._nx_graph)
+        qubits_payload = sorted(list(self._qubits_set))
 
         return {'qubits': qubits_payload, 'nx_graph': graph_payload}
 
     @classmethod
     def _from_json_dict_(cls, qubits, nx_graph, **kwargs):
-        if qubits == '':
-            qubits = None
-        graph_obj = None
-        if nx_graph != '':
-            graph_obj = nx.readwrite.json_graph.node_link_graph(nx_graph)
+        graph_obj = nx.readwrite.json_graph.node_link_graph(nx_graph)
         return cls(qubits, graph_obj)

cirq-core/cirq/devices/device_test.py

@@ -39,22 +39,26 @@ def test_qid_pairs():
     class RawDevice(cirq.Device):
         pass
 
-    assert RawDevice().qid_pairs() is None
+    with cirq.testing.assert_deprecated('device.metadata', deadline='v0.15', count=1):
+        assert RawDevice().qid_pairs() is None
 
     class QubitFieldDevice(cirq.Device):
         def __init__(self, qubits):
             self.qubits = qubits
 
-    assert len(QubitFieldDevice(cirq.LineQubit.range(10)).qid_pairs()) == 9
-    assert len(QubitFieldDevice(cirq.GridQubit.rect(10, 10)).qid_pairs()) == 180
-    assert len(QubitFieldDevice([cirq.NamedQubit(str(s)) for s in range(10)]).qid_pairs()) == 45
+    with cirq.testing.assert_deprecated('device.metadata', deadline='v0.15', count=3):
 
+        assert len(QubitFieldDevice(cirq.LineQubit.range(10)).qid_pairs()) == 9
+        assert len(QubitFieldDevice(cirq.GridQubit.rect(10, 10)).qid_pairs()) == 180
+        assert len(QubitFieldDevice([cirq.NamedQubit(str(s)) for s in range(10)]).qid_pairs()) == 45
 
-def test_qid_pair():
+
+def test_qid_pair_deprecated():
     q0, q1, q2, q3 = cirq.LineQubit.range(4)
-    e1 = cirq.SymmetricalQidPair(q0, q1)
-    e2 = cirq.SymmetricalQidPair(q1, q0)
-    e3 = cirq.SymmetricalQidPair(q2, q3)
+    with cirq.testing.assert_deprecated('device.metadata', deadline='v0.15', count=3):
+        e1 = cirq.SymmetricalQidPair(q0, q1)
+        e2 = cirq.SymmetricalQidPair(q1, q0)
+        e3 = cirq.SymmetricalQidPair(q2, q3)
     assert e1 == e2
     assert e2 != e3
     assert repr(e1) == "cirq.QidPair(cirq.LineQubit(0), cirq.LineQubit(1))"
@@ -74,8 +78,9 @@ def test_qid_pair():
     assert len(set1) == 1
     assert len(set2) == 2
 
-    with pytest.raises(ValueError, match='A QidPair cannot have identical qids.'):
-        cirq.SymmetricalQidPair(q0, q0)
+    with cirq.testing.assert_deprecated('device.metadata', deadline='v0.15', count=1):
+        with pytest.raises(ValueError, match='A QidPair cannot have identical qids.'):
+            cirq.SymmetricalQidPair(q0, q0)
 
 
 def test_device_metadata():
@@ -92,10 +97,6 @@ def test_metadata():
     assert metadata.qubit_set == frozenset(qubits)
     assert metadata.nx_graph == graph
 
-    metadata = cirq.DeviceMetadata()
-    assert metadata.qubit_set is None
-    assert metadata.nx_graph is None
-
 
 def test_metadata_json_load_logic():
     qubits = cirq.LineQubit.range(4)
@@ -104,13 +105,6 @@ def test_metadata_json_load_logic():
     str_rep = cirq.to_json(metadata)
     assert metadata == cirq.read_json(json_text=str_rep)
 
-    qubits = None
-    graph = None
-    metadata = cirq.DeviceMetadata(qubits, graph)
-    str_rep = cirq.to_json(metadata)
-    output = cirq.read_json(json_text=str_rep)
-    assert metadata == output
-
 
 def test_metadata_equality():
     qubits = cirq.LineQubit.range(4)
@@ -121,8 +115,5 @@ def test_metadata_equality():
 
     eq = cirq.testing.EqualsTester()
     eq.add_equality_group(cirq.DeviceMetadata(qubits, graph))
-    eq.add_equality_group(cirq.DeviceMetadata(None, graph))
-    eq.add_equality_group(cirq.DeviceMetadata(qubits, None))
-    eq.add_equality_group(cirq.DeviceMetadata(None, None))
-
-    assert cirq.DeviceMetadata(None, graph) != cirq.DeviceMetadata(None, graph2)
+    eq.add_equality_group(cirq.DeviceMetadata(qubits, graph2))
+    eq.add_equality_group(cirq.DeviceMetadata(qubits[1:], graph))

cirq-core/cirq/ion/ion_device.py

@@ -13,8 +13,7 @@
 # limitations under the License.
 
 from typing import Any, FrozenSet, Iterable, Optional, Set, TYPE_CHECKING
-
-from cirq import circuits, value, devices, ops, protocols
+from cirq import _compat, circuits, value, devices, ops, protocols
 from cirq.ion import convert_to_ion_gates
 
 if TYPE_CHECKING:
@@ -74,14 +73,19 @@ def __init__(
     def qubit_set(self) -> FrozenSet['cirq.LineQubit']:
         return self.qubits
 
+    @_compat.deprecated(
+        deadline='v0.15',
+        fix='qubit coupling data can now be found in device.metadata if provided.',
+    )
     def qid_pairs(self) -> FrozenSet['cirq.SymmetricalQidPair']:
         """Qubits have all-to-all connectivity, so returns all pairs.
 
         Returns:
             All qubit pairs on the device.
         """
-        qs = self.qubits
-        return frozenset([devices.SymmetricalQidPair(q, q2) for q in qs for q2 in qs if q < q2])
+        with _compat.block_overlapping_deprecation('device\\.metadata'):
+            qs = self.qubits
+            return frozenset([devices.SymmetricalQidPair(q, q2) for q in qs for q2 in qs if q < q2])
 
     def decompose_operation(self, operation: ops.Operation) -> ops.OP_TREE:
         return convert_to_ion_gates.ConvertToIonGates().convert_one(operation)

cirq-core/cirq/ion/ion_device_test.py

@@ -211,5 +211,6 @@ def test_qubit_set():
     assert ion_device(3).qubit_set() == frozenset(cirq.LineQubit.range(3))
 
 
-def test_qid_pairs():
-    assert len(ion_device(10).qid_pairs()) == 45
+def test_qid_pairs_deprecated():
+    with cirq.testing.assert_deprecated('device.metadata', deadline='v0.15', count=1):
+        assert len(ion_device(10).qid_pairs()) == 45

cirq-core/cirq/protocols/json_test_data/spec.py

@@ -184,7 +184,10 @@
         'Unique',
         'DEFAULT_RESOLVERS',
     ],
-    deprecated={'GlobalPhaseOperation': 'v0.16'},
+    deprecated={
+        'GlobalPhaseOperation': 'v0.16',
+        'SymmetricalQidPair': 'v0.15',
+    },
     tested_elsewhere=[
         # SerializableByKey does not follow common serialization rules.
         # It is tested separately in test_context_serialization.

cirq-google/cirq_google/devices/serializable_device.py

@@ -26,8 +26,8 @@
     Type,
     FrozenSet,
 )
-
 import cirq
+from cirq import _compat
 from cirq_google.serialization import serializable_gate_set
 from cirq_google.api import v2
 
@@ -259,23 +259,28 @@ def __str__(self) -> str:
 
         return super().__str__()
 
+    @_compat.deprecated(
+        deadline='v0.15',
+        fix='qubit coupling data can now be found in device.metadata if provided.',
+    )
     def qid_pairs(self) -> FrozenSet['cirq.SymmetricalQidPair']:
         """Returns a list of qubit edges on the device, defined by the gate
         definitions.
 
         Returns:
             The list of qubit edges on the device.
         """
-        return frozenset(
-            [
-                cirq.SymmetricalQidPair(pair[0], pair[1])
-                for gate_defs in self.gate_definitions.values()
-                for gate_def in gate_defs
-                if gate_def.number_of_qubits == 2
-                for pair in gate_def.target_set
-                if len(pair) == 2 and pair[0] < pair[1]
-            ]
-        )
+        with _compat.block_overlapping_deprecation('device\\.metadata'):
+            return frozenset(
+                [
+                    cirq.SymmetricalQidPair(pair[0], pair[1])
+                    for gate_defs in self.gate_definitions.values()
+                    for gate_def in gate_defs
+                    if gate_def.number_of_qubits == 2
+                    for pair in gate_def.target_set
+                    if len(pair) == 2 and pair[0] < pair[1]
+                ]
+            )
 
     def _repr_pretty_(self, p: Any, cycle: bool) -> None:
         """Creates ASCII diagram for Jupyter, IPython, etc."""