From 7a9c4caceb8baff89b09019a08d015fb4e83267f Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Tue, 28 Jan 2020 18:19:07 -0500 Subject: [PATCH 01/27] Start conversion of networkx to retworkx This commit is the start of the conversion from using networkx as the internal structure of the dagcircuit to use retworkx instead. --- qiskit/dagcircuit/dagcircuit.py | 210 +++++++++++++++++--------------- requirements.txt | 1 + setup.py | 1 + test/python/test_dagcircuit.py | 31 +++-- tox.ini | 1 + 5 files changed, 127 insertions(+), 117 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 616bd6f1ed20..94f78daac795 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -25,7 +25,7 @@ from collections import OrderedDict import copy import itertools -import networkx as nx +import retworkx as nx from qiskit.circuit.quantumregister import QuantumRegister, Qubit from qiskit.circuit.classicalregister import ClassicalRegister, Clbit @@ -61,9 +61,6 @@ def __init__(self): # Map from wire (Register,idx) to output nodes of the graph self.output_map = OrderedDict() - # Stores the max id of a node added to the DAG - self._max_node_id = 0 - # Directed multigraph whose nodes are inputs, outputs, or operations. # Operation nodes have equal in- and out-degrees and carry # additional data about the operation, including the argument order @@ -71,7 +68,7 @@ def __init__(self): # Input nodes have out-degree 1 and output nodes have in-degree 1. # Edges carry wire labels (reg,idx) and each operation has # corresponding in- and out-edges with the same wire labels. - self._multi_graph = nx.MultiDiGraph() + self._multi_graph = nx.PyDAG() # Map of qreg name to QuantumRegister object self.qregs = OrderedDict() @@ -81,10 +78,11 @@ def __init__(self): # TO REMOVE WHEN NODE IS HAVE BEEN REMOVED FULLY self._id_to_node = {} + self._node_to_id = {} def to_networkx(self): """Returns a copy of the DAGCircuit in networkx format.""" - return copy.deepcopy(self._multi_graph) + return copy.copy(self._multi_graph) def qubits(self): """Return a list of qubits (as a list of Qubit instances).""" @@ -138,34 +136,27 @@ def _add_wire(self, wire): """ if wire not in self.wires: self.wires.append(wire) - self._max_node_id += 1 - input_map_wire = self.input_map[wire] = self._max_node_id - - self._max_node_id += 1 - output_map_wire = self._max_node_id wire_name = "%s[%s]" % (wire.register.name, wire.index) - inp_node = DAGNode(data_dict={'type': 'in', 'name': wire_name, 'wire': wire}, - nid=input_map_wire) - outp_node = DAGNode(data_dict={'type': 'out', 'name': wire_name, 'wire': wire}, - nid=output_map_wire) + inp_node = DAGNode(data_dict={'type': 'in', 'name': wire_name, 'wire': wire}) + outp_node = DAGNode(data_dict={'type': 'out', 'name': wire_name, 'wire': wire}) + input_map_wire = self._multi_graph.add_node(inp_node) + output_map_wire = self._multi_graph.add_node(outp_node) + inp_node._node_id = input_map_wire + outp_node._node_id = output_map_wire self._id_to_node[input_map_wire] = inp_node self._id_to_node[output_map_wire] = outp_node - + self._node_to_id[inp_node] = input_map_wire + self._node_to_id[outp_node] = output_map_wire self.input_map[wire] = inp_node self.output_map[wire] = outp_node - self._multi_graph.add_node(inp_node) - self._multi_graph.add_node(outp_node) - - self._multi_graph.add_edge(inp_node, - outp_node) - self._multi_graph.adj[inp_node][outp_node][0]["name"] \ - = "%s[%s]" % (wire.register.name, wire.index) - self._multi_graph.adj[inp_node][outp_node][0]["wire"] \ - = wire + self._multi_graph.add_edge(inp_node._node_id, + outp_node._node_id, + {'name': "%s[%s]" % (wire.register.name, wire.index), + 'wire': wire}) else: raise DAGCircuitError("duplicate wire %s" % (wire,)) @@ -231,10 +222,12 @@ def _add_op_node(self, op, qargs, cargs, condition=None): } # Add a new operation node to the graph - self._max_node_id += 1 - new_node = DAGNode(data_dict=node_properties, nid=self._max_node_id) - self._multi_graph.add_node(new_node) - self._id_to_node[self._max_node_id] = new_node + new_node = DAGNode(data_dict=node_properties) + node_index = self._multi_graph.add_node(new_node) + new_node._node_id = node_index + self._id_to_node[node_index] = new_node + self._node_to_id[new_node] = node_index + return node_index def apply_operation_back(self, op, qargs=None, cargs=None, condition=None): """Apply an operation to the output of the circuit. @@ -262,25 +255,26 @@ def apply_operation_back(self, op, qargs=None, cargs=None, condition=None): self._check_bits(qargs, self.output_map) self._check_bits(all_cbits, self.output_map) - self._add_op_node(op, qargs, cargs, condition) + node_index = self._add_op_node(op, qargs, cargs, condition) # Add new in-edges from predecessors of the output nodes to the # operation node while deleting the old in-edges of the output nodes # and adding new edges from the operation node to each output node al = [qargs, all_cbits] for q in itertools.chain(*al): - ie = list(self._multi_graph.predecessors(self.output_map[q])) + ie = list(self._multi_graph.predecessors(self.output_map[q]._node_id)) if len(ie) != 1: raise DAGCircuitError("output node has multiple in-edges") - self._multi_graph.add_edge(ie[0], self._id_to_node[self._max_node_id], - name="%s[%s]" % (q.register.name, q.index), wire=q) - self._multi_graph.remove_edge(ie[0], self.output_map[q]) - self._multi_graph.add_edge(self._id_to_node[self._max_node_id], self.output_map[q], - name="%s[%s]" % (q.register.name, q.index), wire=q) + self._multi_graph.add_edge(ie[0]._node_id, node_index, + {'name': "%s[%s]" % (q.register.name, q.index), 'wire': q}) - return self._id_to_node[self._max_node_id] + self._multi_graph.remove_edge(ie[0]._node_id, self.output_map[q]._node_id) + self._multi_graph.add_edge(node_index, self.output_map[q]._node_id, + dict(name="%s[%s]" % (q.register.name, q.index), wire=q)) + + return self._id_to_node[node_index] def apply_operation_front(self, op, qargs, cargs, condition=None): """Apply an operation to the input of the circuit. @@ -303,22 +297,22 @@ def apply_operation_front(self, op, qargs, cargs, condition=None): self._check_condition(op.name, condition) self._check_bits(qargs, self.input_map) self._check_bits(all_cbits, self.input_map) - self._add_op_node(op, qargs, cargs, condition) + node_index = self._add_op_node(op, qargs, cargs, condition) # Add new out-edges to successors of the input nodes from the # operation node while deleting the old out-edges of the input nodes # and adding new edges to the operation node from each input node al = [qargs, all_cbits] for q in itertools.chain(*al): - ie = list(self._multi_graph.successors(self.input_map[q])) + ie = list(self._multi_graph.successors(self.input_map[q]._node_id)) if len(ie) != 1: raise DAGCircuitError("input node has multiple out-edges") - self._multi_graph.add_edge(self._id_to_node[self._max_node_id], ie[0], - name="%s[%s]" % (q.register.name, q.index), wire=q) - self._multi_graph.remove_edge(self.input_map[q], ie[0]) - self._multi_graph.add_edge(self.input_map[q], self._id_to_node[self._max_node_id], - name="%s[%s]" % (q.register.name, q.index), wire=q) + self._multi_graph.add_edge(node_index, ie[0]._node_id, + dict(name="%s[%s]" % (q.register.name, q.index), wire=q)) + self._multi_graph.remove_edge(self.input_map[q]._node_id, ie[0]._node_id) + self._multi_graph.add_edge(self.input_map[q]._node_id, node_index, + dict(name="%s[%s]" % (q.register.name, q.index), wire=q)) - return self._id_to_node[self._max_node_id] + return self._id_to_node[node_index] def _check_edgemap_registers(self, edge_map, keyregs, valregs, valreg=True): """Check that wiremap neither fragments nor leaves duplicate registers. @@ -574,7 +568,7 @@ def idle_wires(self): def size(self): """Return the number of operations.""" - return self._multi_graph.order() - 2 * len(self.wires) + return len(self._multi_graph) - 2 * len(self.wires) def depth(self): """Return the circuit depth. @@ -587,7 +581,7 @@ def depth(self): raise DAGCircuitError("not a DAG") depth = nx.dag_longest_path_length(self._multi_graph) - 1 - return depth if depth != -1 else 0 + return depth if depth >= 0 else 0 def width(self): """Return the total number of qubits + clbits used by the circuit. @@ -654,9 +648,9 @@ def _make_pred_succ_maps(self, node): """ pred_map = {e[2]['wire']: e[0] for e in - self._multi_graph.in_edges(nbunch=node, data=True)} + self._multi_graph.in_edges(node._node_id)} succ_map = {e[2]['wire']: e[1] for e in - self._multi_graph.out_edges(nbunch=node, data=True)} + self._multi_graph.out_edges(node._node_id)} return pred_map, succ_map def _full_pred_succ_maps(self, pred_map, succ_map, input_circuit, @@ -699,17 +693,17 @@ def _full_pred_succ_maps(self, pred_map, succ_map, input_circuit, return full_pred_map, full_succ_map def __eq__(self, other): - # TODO this works but is a horrible way to do this - slf = copy.deepcopy(self._multi_graph) - oth = copy.deepcopy(other._multi_graph) + slf = self._multi_graph + oth = other._multi_graph - for node in slf.nodes: - slf.nodes[node]['node'] = node - for node in oth.nodes: - oth.nodes[node]['node'] = node +# for node in slf.nodes: +# slf.nodes[node]['node'] = node +# for node in oth.nodes: +# oth.nodes[node]['node'] = node - return nx.is_isomorphic(slf, oth, - node_match=lambda x, y: DAGNode.semantic_eq(x['node'], y['node'])) + return nx.is_isomorphic_node_match(slf, oth, + lambda x, y: DAGNode.semantic_eq( + x, y)) def topological_nodes(self): """ @@ -718,7 +712,7 @@ def topological_nodes(self): Returns: generator(DAGNode): node in topological order """ - return nx.lexicographical_topological_sort(self._multi_graph, key=lambda x: str(x.qargs)) + return [self._multi_graph.get_node_data(x) for x in nx.topological_sort(self._multi_graph)] def topological_op_nodes(self): """ @@ -811,7 +805,7 @@ def substitute_node_with_dag(self, node, input_dag, wires=None): 'on which it would be conditioned.') # Now that we know the connections, delete node - self._multi_graph.remove_node(node) + self._multi_graph.remove_node(node._node_id) # Iterate over nodes of input_circuit for sorted_node in input_dag.topological_op_nodes(): @@ -821,7 +815,7 @@ def substitute_node_with_dag(self, node, input_dag, wires=None): sorted_node.qargs)) m_cargs = list(map(lambda x: wire_map.get(x, x), sorted_node.cargs)) - self._add_op_node(sorted_node.op, m_qargs, m_cargs, condition) + node_index = self._add_op_node(sorted_node.op, m_qargs, m_cargs, condition) # Add edges from predecessor nodes to new node # and update predecessor nodes that change all_cbits = self._bits_in_condition(condition) @@ -829,19 +823,19 @@ def substitute_node_with_dag(self, node, input_dag, wires=None): al = [m_qargs, all_cbits] for q in itertools.chain(*al): self._multi_graph.add_edge(full_pred_map[q], - self._id_to_node[self._max_node_id], - name="%s[%s]" % (q.register.name, q.index), - wire=q) - full_pred_map[q] = self._id_to_node[self._max_node_id] + node_index, + dict(name="%s[%s]" % (q.register.name, q.index), + wire=q)) + full_pred_map[q] = node_index # Connect all predecessors and successors, and remove # residual edges between input and output nodes for w in full_pred_map: self._multi_graph.add_edge(full_pred_map[w], full_succ_map[w], - name="%s[%s]" % (w.register.name, w.index), - wire=w) - o_pred = list(self._multi_graph.predecessors(self.output_map[w])) + dict(name="%s[%s]" % (w.register.name, w.index), + wire=w)) + o_pred = list(self._multi_graph.predecessors(self.output_map[w]._node_id)) if len(o_pred) > 1: if len(o_pred) != 2: raise DAGCircuitError("expected 2 predecessors here") @@ -891,23 +885,24 @@ def substitute_node(self, node, op, inplace=False): node.data_dict['name'] = op.name return node - self._max_node_id += 1 new_data_dict = node.data_dict.copy() new_data_dict['op'] = op new_data_dict['name'] = op.name - new_node = DAGNode(new_data_dict, nid=self._max_node_id) + new_node = DAGNode(new_data_dict) - self._multi_graph.add_node(new_node) + node_index = self._multi_graph.add_node(new_node) + new_node._node_id = node_index + self._id_to_node[node_index] = new_node - in_edges = self._multi_graph.in_edges(node, data=True) - out_edges = self._multi_graph.out_edges(node, data=True) + in_edges = self._multi_graph.in_edges(node._node_id) + out_edges = self._multi_graph.out_edges(node._node_id) - self._multi_graph.add_edges_from( - [(src, new_node, data) for src, dest, data in in_edges]) - self._multi_graph.add_edges_from( - [(new_node, dest, data) for src, dest, data in out_edges]) + for src, _, data in in_edges: + self._multi_graph.add_edge(src, node_index, data) + for _, dest, data in out_edges: + self._multi_graph.add_edge(node_index, dest, data) - self._multi_graph.remove_node(node) + self._multi_graph.remove_node(node._node_id) return new_node @@ -920,7 +915,7 @@ def node(self, node_id): Returns: node: the node. """ - return self._multi_graph.nodes[node_id] + return self._multi_graph.get_node_data(node_id) def nodes(self): """Iterator for node values. @@ -928,17 +923,24 @@ def nodes(self): Yield: node: the node. """ - for node in self._multi_graph.nodes: + for node in self._multi_graph.nodes(): yield node def edges(self, nodes=None): - """Iterator for node values. + """Iterator for edge values and source and dest node Yield: - node: the node. + edge: the edge. """ - for source_node, dest_node, edge_data in self._multi_graph.edges(nodes, data=True): - yield source_node, dest_node, edge_data + if nodes is None: + nodes = self._multi_graph.nodes() + elif isinstance(nodes, DAGNode): + nodes = [nodes] + for node in nodes: + in_list = self._multi_graph.in_edges(node._node_id) + out_list = self._multi_graph.out_edges(node._node_id) + for x in in_list + out_list: + yield x def op_nodes(self, op=None): """Get the list of "op" nodes in the dag. @@ -998,26 +1000,26 @@ def longest_path(self): def successors(self, node): """Returns iterator of the successors of a node as DAGNodes.""" - return self._multi_graph.successors(node) + return self._multi_graph.successors(node._node_id) def predecessors(self, node): """Returns iterator of the predecessors of a node as DAGNodes.""" - return self._multi_graph.predecessors(node) + return self._multi_graph.predecessors(node._node_id) def quantum_predecessors(self, node): """Returns iterator of the predecessors of a node that are connected by a quantum edge as DAGNodes.""" - for predecessor in self.predecessors(node): - if isinstance(self._multi_graph.get_edge_data(predecessor, node, key=0)['wire'], Qubit): + for predecessor in reversed(self.predecessors(node)): + if isinstance(self._multi_graph.get_edge_data(predecessor._node_id, node._node_id)['wire'], Qubit): yield predecessor def ancestors(self, node): """Returns set of the ancestors of a node as DAGNodes.""" - return nx.ancestors(self._multi_graph, node) + return set(self._id_to_node[x] for x in nx.ancestors(self._multi_graph, node._node_id)) def descendants(self, node): """Returns set of the descendants of a node as DAGNodes.""" - return nx.descendants(self._multi_graph, node) + return set(self._id_to_node[x] for x in nx.descendants(self._multi_graph, node._node_id)) def bfs_successors(self, node): """ @@ -1029,9 +1031,9 @@ def bfs_successors(self, node): def quantum_successors(self, node): """Returns iterator of the successors of a node that are connected by a quantum edge as DAGNodes.""" - for successor in self.successors(node): + for successor in reversed(self.successors(node)): if isinstance(self._multi_graph.get_edge_data( - node, successor, key=0)['wire'], Qubit): + node._node_id, successor._node_id)['wire'], Qubit): yield successor def remove_op_node(self, node): @@ -1046,11 +1048,11 @@ def remove_op_node(self, node): pred_map, succ_map = self._make_pred_succ_maps(node) # remove from graph and map - self._multi_graph.remove_node(node) + self._multi_graph.remove_node(node._node_id) for w in pred_map.keys(): self._multi_graph.add_edge(pred_map[w], succ_map[w], - name="%s[%s]" % (w.register.name, w.index), wire=w) + dict(name="%s[%s]" % (w.register.name, w.index), wire=w)) def remove_ancestors_of(self, node): """Remove all of the ancestor operation nodes of node.""" @@ -1189,13 +1191,13 @@ def multigraph_layers(self): while cur_layer: for node in cur_layer: # Count multiedges with multiplicity. - for successor in self._multi_graph.successors(node): - multiplicity = self._multi_graph.number_of_edges(node, successor) + for successor in self._multi_graph.successors(node._node_id): + multiplicity = len(self._multi_graph.get_all_edge_data(node._node_id, successor._node_id)) if successor in predecessor_count: predecessor_count[successor] -= multiplicity else: predecessor_count[successor] = \ - self._multi_graph.in_degree(successor) - multiplicity + self._multi_graph.in_degree(successor._node_id) - multiplicity if predecessor_count[successor] == 0: next_layer.append(successor) @@ -1229,14 +1231,14 @@ def collect_runs(self, namelist): and not nodes_seen[node]: group = [node] nodes_seen[node] = True - s = list(self._multi_graph.successors(node)) + s = self._multi_graph.successors(node._node_id) while len(s) == 1 and \ s[0].type == "op" and \ s[0].name in namelist and \ s[0].condition is None: group.append(s[0]) nodes_seen[s[0]] = True - s = list(self._multi_graph.successors(s[0])) + s = list(self._multi_graph.successors(s[0]._node_id)) if len(group) >= 1: group_list.append(tuple(group)) return set(group_list) @@ -1269,8 +1271,14 @@ def nodes_on_wire(self, wire, only_ops=False): yield current_node # find the adjacent node that takes the wire being looked at as input - for node, edges in self._multi_graph.adj[current_node].items(): - if any(wire == edge['wire'] for edge in edges.values()): + # TODO(mtreinish): Add function in retworkx that does this nested api + for node_index in self._multi_graph.adj(current_node._node_id): + node = self._id_to_node[node_index] + try: + edge_data = self._multi_graph.get_all_edge_data(current_node._node_id, node_index) + except Exception: + edge_data = self._multi_graph.get_all_edge_data(node_index, current_node._node_id) + if any(wire == edge['wire'] for edge in edge_data): current_node = node more_nodes = True break diff --git a/requirements.txt b/requirements.txt index 290286ccd736..b58707c38c4e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,6 +3,7 @@ marshmallow>=3,<4 marshmallow_polyfield>=5.7,<6 networkx>=2.2;python_version>'3.5' networkx>=2.2,<2.4;python_version=='3.5' +retworkx>=0.1.0 numpy>=1.13 ply>=3.10 psutil>=5 diff --git a/setup.py b/setup.py index 5cc33a01118f..007c8efc4bc2 100755 --- a/setup.py +++ b/setup.py @@ -31,6 +31,7 @@ "networkx>=2.2;python_version>'3.5'", # Networkx 2.4 is the final version with python 3.5 support. "networkx>=2.2,<2.4;python_version=='3.5'", + "retworkx>=0.1.0", "numpy>=1.13", "ply>=3.10", "psutil>=5", diff --git a/test/python/test_dagcircuit.py b/test/python/test_dagcircuit.py index 76fb01ac7f0c..9bcd45651701 100644 --- a/test/python/test_dagcircuit.py +++ b/test/python/test_dagcircuit.py @@ -18,7 +18,7 @@ from ddt import ddt, data -import networkx as nx +import retworkx as nx from qiskit.dagcircuit import DAGCircuit from qiskit.circuit import QuantumRegister @@ -72,8 +72,8 @@ def raise_if_dagcircuit_invalid(dag): # Every edge should be labled with a known wire. edges_outside_wires = [edge_data['wire'] - for source, dest, edge_data - in multi_graph.edges(data=True) + for edge_data + in multi_graph.edges() if edge_data['wire'] not in dag.wires] if edges_outside_wires: raise DAGCircuitError('multi_graph contains one or more edges ({}) ' @@ -94,23 +94,22 @@ def raise_if_dagcircuit_invalid(dag): for wire in dag.wires: cur_node = dag.input_map[wire] out_node = dag.output_map[wire] - while cur_node != out_node: - out_edges = multi_graph.out_edges(cur_node, data=True) + out_edges = multi_graph.out_edges(cur_node._node_id) edges_to_follow = [(src, dest, data) for (src, dest, data) in out_edges if data['wire'] == wire] assert len(edges_to_follow) == 1 - cur_node = edges_to_follow[0][1] + cur_node = dag._id_to_node[edges_to_follow[0][1]] # Wires can only terminate at input/output nodes. for op_node in dag.op_nodes(): - assert multi_graph.in_degree(op_node) == multi_graph.out_degree(op_node) + assert multi_graph.in_degree(op_node._node_id) == multi_graph.out_degree(op_node._node_id) # Node input/output edges should match node qarg/carg/condition. for node in dag.op_nodes(): - in_edges = multi_graph.in_edges(node, data=True) - out_edges = multi_graph.out_edges(node, data=True) + in_edges = multi_graph.in_edges(node._node_id) + out_edges = multi_graph.out_edges(node._node_id) in_wires = {data['wire'] for src, dest, data in in_edges} out_wires = {data['wire'] for src, dest, data in out_edges} @@ -219,7 +218,7 @@ def test_apply_operation_back_conditional(self): self.assertEqual(h_node.condition, h_gate.condition) self.assertEqual( - sorted(self.dag._multi_graph.in_edges(h_node, data=True)), + sorted(self.dag._multi_graph.in_edges(h_node._node_id)), sorted([ (self.dag.input_map[self.qubit2], h_node, {'wire': self.qubit2, 'name': 'qr[2]'}), @@ -230,7 +229,7 @@ def test_apply_operation_back_conditional(self): ])) self.assertEqual( - sorted(self.dag._multi_graph.out_edges(h_node, data=True)), + sorted(self.dag._multi_graph.out_edges(h_node._node_id)), sorted([ (h_node, self.dag.output_map[self.qubit2], {'wire': self.qubit2, 'name': 'qr[2]'}), @@ -261,7 +260,7 @@ def test_apply_operation_back_conditional_measure(self): self.assertEqual(meas_node.condition, meas_gate.condition) self.assertEqual( - sorted(self.dag._multi_graph.in_edges(meas_node, data=True)), + sorted(self.dag._multi_graph.in_edges(meas_node._node_id)), sorted([ (self.dag.input_map[self.qubit0], meas_node, {'wire': self.qubit0, 'name': 'qr[0]'}), @@ -272,7 +271,7 @@ def test_apply_operation_back_conditional_measure(self): ])) self.assertEqual( - sorted(self.dag._multi_graph.out_edges(meas_node, data=True)), + sorted(self.dag._multi_graph.out_edges(meas_node._node_id)), sorted([ (meas_node, self.dag.output_map[self.qubit0], {'wire': self.qubit0, 'name': 'qr[0]'}), @@ -300,7 +299,7 @@ def test_apply_operation_back_conditional_measure_to_self(self): self.assertEqual(meas_node.condition, meas_gate.condition) self.assertEqual( - sorted(self.dag._multi_graph.in_edges(meas_node, data=True)), + sorted(self.dag._multi_graph.in_edges(meas_node._node_id)), sorted([ (self.dag.input_map[self.qubit1], meas_node, {'wire': self.qubit1, 'name': 'qr[1]'}), @@ -311,7 +310,7 @@ def test_apply_operation_back_conditional_measure_to_self(self): ])) self.assertEqual( - sorted(self.dag._multi_graph.out_edges(meas_node, data=True)), + sorted(self.dag._multi_graph.out_edges(meas_node)), sorted([ (meas_node, self.dag.output_map[self.qubit1], {'wire': self.qubit1, 'name': 'qr[1]'}), @@ -565,7 +564,7 @@ def test_remove_non_op_node(self): """Try to remove a non-op node with remove_op_node method.""" self.dag.apply_operation_back(HGate(), [self.qubit0]) - in_node = next(self.dag.topological_nodes()) + in_node = next(iter(self.dag.topological_nodes())) self.assertRaises(DAGCircuitError, self.dag.remove_op_node, in_node) def test_dag_collect_runs(self): diff --git a/tox.ini b/tox.ini index 9730a0fb797e..f9b83756565e 100644 --- a/tox.ini +++ b/tox.ini @@ -11,6 +11,7 @@ setenv = LANGUAGE=en_US LC_ALL=en_US.utf-8 ARGS="-V" + RUST_BACKTRACE=full deps = -r{toxinidir}/requirements.txt -r{toxinidir}/requirements-dev.txt commands = From fde9f74fb3ee108d47ba4da6ea0627114c562808 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 29 Jan 2020 12:16:03 -0500 Subject: [PATCH 02/27] Implement to_networkx() method The to_networkx() method is used to return a networkx graph object representing the internal dag structure. Since the previous commit switched this to retwork's PyDAG class this function was broken. To enable the continued function of networkx based utilities (mainly the dag drawer) this commit adds an implementation that iterates over PyDAG twice (so it's not super efficient at O(2n)) to add the nodes and edges. While this isn't the most effecient algorithm it works. To speed it up in the future a function can be added to retworkx to either directly generate the networkx object or alternatively generate the parameters so it can be passed to networkx in a single call. --- qiskit/dagcircuit/dagcircuit.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 94f78daac795..28344527c782 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -26,6 +26,7 @@ import copy import itertools import retworkx as nx +import networkx from qiskit.circuit.quantumregister import QuantumRegister, Qubit from qiskit.circuit.classicalregister import ClassicalRegister, Clbit @@ -82,7 +83,14 @@ def __init__(self): def to_networkx(self): """Returns a copy of the DAGCircuit in networkx format.""" - return copy.copy(self._multi_graph) + G = networkx.MultiDiGraph() + for node in self._multi_graph.nodes(): + G.add_node(node) + for node in nx.topological_sort(self._multi_graph): + for source, dest, edge in self._multi_graph.in_edges(node): + G.add_edge(self._id_to_node[source], self._id_to_node[dest], + **edge) + return G def qubits(self): """Return a list of qubits (as a list of Qubit instances).""" From c4d3fb38b27917921de26e57ae0815baa7a815f2 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 31 Jan 2020 13:42:19 -0500 Subject: [PATCH 03/27] Use lexicographical_topological_sort() --- qiskit/dagcircuit/dagcircuit.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 28344527c782..076e5c14721f 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -720,7 +720,8 @@ def topological_nodes(self): Returns: generator(DAGNode): node in topological order """ - return [self._multi_graph.get_node_data(x) for x in nx.topological_sort(self._multi_graph)] + return iter(nx.lexicographical_topological_sort( + self._multi_graph, key=lambda x: str(x.qargs))) def topological_op_nodes(self): """ From 326e85477039b05bccd0c5d52cffbb500561fc55 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 31 Jan 2020 15:35:49 -0500 Subject: [PATCH 04/27] More fixes --- qiskit/dagcircuit/dagcircuit.py | 66 +++++++++++++++++---------------- test/python/test_dagcircuit.py | 4 +- 2 files changed, 37 insertions(+), 33 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 076e5c14721f..0786645a285a 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -25,7 +25,7 @@ from collections import OrderedDict import copy import itertools -import retworkx as nx +import retworkx as rx import networkx from qiskit.circuit.quantumregister import QuantumRegister, Qubit @@ -69,7 +69,7 @@ def __init__(self): # Input nodes have out-degree 1 and output nodes have in-degree 1. # Edges carry wire labels (reg,idx) and each operation has # corresponding in- and out-edges with the same wire labels. - self._multi_graph = nx.PyDAG() + self._multi_graph = rx.PyDAG() # Map of qreg name to QuantumRegister object self.qregs = OrderedDict() @@ -79,14 +79,13 @@ def __init__(self): # TO REMOVE WHEN NODE IS HAVE BEEN REMOVED FULLY self._id_to_node = {} - self._node_to_id = {} def to_networkx(self): """Returns a copy of the DAGCircuit in networkx format.""" G = networkx.MultiDiGraph() for node in self._multi_graph.nodes(): G.add_node(node) - for node in nx.topological_sort(self._multi_graph): + for node in rx.topological_sort(self._multi_graph): for source, dest, edge in self._multi_graph.in_edges(node): G.add_edge(self._id_to_node[source], self._id_to_node[dest], **edge) @@ -155,8 +154,6 @@ def _add_wire(self, wire): outp_node._node_id = output_map_wire self._id_to_node[input_map_wire] = inp_node self._id_to_node[output_map_wire] = outp_node - self._node_to_id[inp_node] = input_map_wire - self._node_to_id[outp_node] = output_map_wire self.input_map[wire] = inp_node self.output_map[wire] = outp_node @@ -234,7 +231,6 @@ def _add_op_node(self, op, qargs, cargs, condition=None): node_index = self._multi_graph.add_node(new_node) new_node._node_id = node_index self._id_to_node[node_index] = new_node - self._node_to_id[new_node] = node_index return node_index def apply_operation_back(self, op, qargs=None, cargs=None, condition=None): @@ -275,12 +271,14 @@ def apply_operation_back(self, op, qargs=None, cargs=None, condition=None): if len(ie) != 1: raise DAGCircuitError("output node has multiple in-edges") - self._multi_graph.add_edge(ie[0]._node_id, node_index, - {'name': "%s[%s]" % (q.register.name, q.index), 'wire': q}) + self._multi_graph.add_edge( + ie[0]._node_id, node_index, + {'name': "%s[%s]" % (q.register.name, q.index), 'wire': q}) self._multi_graph.remove_edge(ie[0]._node_id, self.output_map[q]._node_id) - self._multi_graph.add_edge(node_index, self.output_map[q]._node_id, - dict(name="%s[%s]" % (q.register.name, q.index), wire=q)) + self._multi_graph.add_edge( + node_index, self.output_map[q]._node_id, + dict(name="%s[%s]" % (q.register.name, q.index), wire=q)) return self._id_to_node[node_index] @@ -585,10 +583,10 @@ def depth(self): Raises: DAGCircuitError: if not a directed acyclic graph """ - if not nx.is_directed_acyclic_graph(self._multi_graph): + if not rx.is_directed_acyclic_graph(self._multi_graph): raise DAGCircuitError("not a DAG") - depth = nx.dag_longest_path_length(self._multi_graph) - 1 + depth = rx.dag_longest_path_length(self._multi_graph) - 1 return depth if depth >= 0 else 0 def width(self): @@ -615,7 +613,7 @@ def num_clbits(self): def num_tensor_factors(self): """Compute how many components the circuit can decompose into.""" - return nx.number_weakly_connected_components(self._multi_graph) + return rx.number_weakly_connected_components(self._multi_graph) def _check_wires_list(self, wires, node): """Check that a list of wires is compatible with a node to be replaced. @@ -709,7 +707,7 @@ def __eq__(self, other): # for node in oth.nodes: # oth.nodes[node]['node'] = node - return nx.is_isomorphic_node_match(slf, oth, + return rx.is_isomorphic_node_match(slf, oth, lambda x, y: DAGNode.semantic_eq( x, y)) @@ -720,7 +718,7 @@ def topological_nodes(self): Returns: generator(DAGNode): node in topological order """ - return iter(nx.lexicographical_topological_sort( + return iter(rx.lexicographical_topological_sort( self._multi_graph, key=lambda x: str(x.qargs))) def topological_op_nodes(self): @@ -943,13 +941,12 @@ def edges(self, nodes=None): """ if nodes is None: nodes = self._multi_graph.nodes() + elif isinstance(nodes, DAGNode): nodes = [nodes] for node in nodes: - in_list = self._multi_graph.in_edges(node._node_id) - out_list = self._multi_graph.out_edges(node._node_id) - for x in in_list + out_list: - yield x + for source, dest, edge in self._multi_graph.in_edges(node._node_id): + yield self._id_to_node[source], self._id_to_node[dest], edge def op_nodes(self, op=None): """Get the list of "op" nodes in the dag. @@ -1005,7 +1002,7 @@ def threeQ_or_more_gates(self): def longest_path(self): """Returns the longest path in the dag as a list of DAGNodes.""" - return nx.dag_longest_path(self._multi_graph) + return rx.dag_longest_path(self._multi_graph) def successors(self, node): """Returns iterator of the successors of a node as DAGNodes.""" @@ -1019,23 +1016,25 @@ def quantum_predecessors(self, node): """Returns iterator of the predecessors of a node that are connected by a quantum edge as DAGNodes.""" for predecessor in reversed(self.predecessors(node)): - if isinstance(self._multi_graph.get_edge_data(predecessor._node_id, node._node_id)['wire'], Qubit): + if isinstance( + self._multi_graph.get_edge_data( + predecessor._node_id, node._node_id)['wire'], Qubit): yield predecessor def ancestors(self, node): """Returns set of the ancestors of a node as DAGNodes.""" - return set(self._id_to_node[x] for x in nx.ancestors(self._multi_graph, node._node_id)) + return set(self._id_to_node[x] for x in rx.ancestors(self._multi_graph, node._node_id)) def descendants(self, node): """Returns set of the descendants of a node as DAGNodes.""" - return set(self._id_to_node[x] for x in nx.descendants(self._multi_graph, node._node_id)) + return set(self._id_to_node[x] for x in rx.descendants(self._multi_graph, node._node_id)) def bfs_successors(self, node): """ Returns an iterator of tuples of (DAGNode, [DAGNodes]) where the DAGNode is the current node and [DAGNode] is its successors in BFS order. """ - return nx.bfs_successors(self._multi_graph, node) + return rx.bfs_successors(self._multi_graph, node) def quantum_successors(self, node): """Returns iterator of the successors of a node that are @@ -1065,7 +1064,7 @@ def remove_op_node(self, node): def remove_ancestors_of(self, node): """Remove all of the ancestor operation nodes of node.""" - anc = nx.ancestors(self._multi_graph, node) + anc = rx.ancestors(self._multi_graph, node) # TODO: probably better to do all at once using # multi_graph.remove_nodes_from; same for related functions ... for anc_node in anc: @@ -1074,14 +1073,14 @@ def remove_ancestors_of(self, node): def remove_descendants_of(self, node): """Remove all of the descendant operation nodes of node.""" - desc = nx.descendants(self._multi_graph, node) + desc = rx.descendants(self._multi_graph, node) for desc_node in desc: if desc_node.type == "op": self.remove_op_node(desc_node) def remove_nonancestors_of(self, node): """Remove all of the non-ancestors operation nodes of node.""" - anc = nx.ancestors(self._multi_graph, node) + anc = rx.ancestors(self._multi_graph, node) comp = list(set(self._multi_graph.nodes()) - set(anc)) for n in comp: if n.type == "op": @@ -1089,7 +1088,7 @@ def remove_nonancestors_of(self, node): def remove_nondescendants_of(self, node): """Remove all of the non-descendants operation nodes of node.""" - dec = nx.descendants(self._multi_graph, node) + dec = rx.descendants(self._multi_graph, node) comp = list(set(self._multi_graph.nodes()) - set(dec)) for n in comp: if n.type == "op": @@ -1201,7 +1200,8 @@ def multigraph_layers(self): for node in cur_layer: # Count multiedges with multiplicity. for successor in self._multi_graph.successors(node._node_id): - multiplicity = len(self._multi_graph.get_all_edge_data(node._node_id, successor._node_id)) + multiplicity = len(self._multi_graph.get_all_edge_data( + node._node_id, successor._node_id)) if successor in predecessor_count: predecessor_count[successor] -= multiplicity else: @@ -1284,9 +1284,11 @@ def nodes_on_wire(self, wire, only_ops=False): for node_index in self._multi_graph.adj(current_node._node_id): node = self._id_to_node[node_index] try: - edge_data = self._multi_graph.get_all_edge_data(current_node._node_id, node_index) + edge_data = self._multi_graph.get_all_edge_data( + current_node._node_id, node_index) except Exception: - edge_data = self._multi_graph.get_all_edge_data(node_index, current_node._node_id) + edge_data = self._multi_graph.get_all_edge_data( + node_index, current_node._node_id) if any(wire == edge['wire'] for edge in edge_data): current_node = node more_nodes = True diff --git a/test/python/test_dagcircuit.py b/test/python/test_dagcircuit.py index 9bcd45651701..1505927b4cfb 100644 --- a/test/python/test_dagcircuit.py +++ b/test/python/test_dagcircuit.py @@ -452,6 +452,7 @@ def test_get_named_nodes(self): def test_topological_nodes(self): """The topological_nodes() method""" + self.maxDiff = None self.dag.apply_operation_back(CnotGate(), [self.qubit0, self.qubit1], []) self.dag.apply_operation_back(HGate(), [self.qubit0], []) self.dag.apply_operation_back(CnotGate(), [self.qubit2, self.qubit1], []) @@ -475,7 +476,8 @@ def test_topological_nodes(self): ('cr[0]', []), ('cr[1]', []), ('cr[1]', [])] - self.assertEqual(expected, [(i.name, i.qargs) for i in named_nodes]) + result = [(i.name, i.qargs) for i in named_nodes] + self.assertEqual(expected, result) def test_topological_op_nodes(self): """The topological_op_nodes() method""" From 9fec1c9c87453f95e1699dff0264c0a2757efecd Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Mon, 3 Feb 2020 15:43:09 -0500 Subject: [PATCH 05/27] Add back deepycopy to __eq__ --- qiskit/dagcircuit/dagcircuit.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 0786645a285a..7f22531e6538 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -699,14 +699,8 @@ def _full_pred_succ_maps(self, pred_map, succ_map, input_circuit, return full_pred_map, full_succ_map def __eq__(self, other): - slf = self._multi_graph - oth = other._multi_graph - -# for node in slf.nodes: -# slf.nodes[node]['node'] = node -# for node in oth.nodes: -# oth.nodes[node]['node'] = node - + slf = copy.deepcopy(self._multi_graph) + oth = copy.deepcopy(other._multi_graph) return rx.is_isomorphic_node_match(slf, oth, lambda x, y: DAGNode.semantic_eq( x, y)) From 19d2a21f36e954bc1444c6be15181ef46381075d Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Mon, 3 Feb 2020 15:49:35 -0500 Subject: [PATCH 06/27] Fix indexing on node_on_wire test --- qiskit/dagcircuit/dagcircuit.py | 1 + test/python/test_dagcircuit.py | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 7f22531e6538..331b170ca19c 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -699,6 +699,7 @@ def _full_pred_succ_maps(self, pred_map, succ_map, input_circuit, return full_pred_map, full_succ_map def __eq__(self, other): + # TODO this works but is a horrible way to do this slf = copy.deepcopy(self._multi_graph) oth = copy.deepcopy(other._multi_graph) return rx.is_isomorphic_node_match(slf, oth, diff --git a/test/python/test_dagcircuit.py b/test/python/test_dagcircuit.py index 1505927b4cfb..6026af5a2ca8 100644 --- a/test/python/test_dagcircuit.py +++ b/test/python/test_dagcircuit.py @@ -502,12 +502,12 @@ def test_dag_nodes_on_wire(self): self.dag.apply_operation_back(HGate(), [self.qubit0], []) qbit = self.dag.qubits()[0] - self.assertEqual([1, 11, 12, 2], [i._node_id for i in self.dag.nodes_on_wire(qbit)]) - self.assertEqual([11, 12], + self.assertEqual([0, 10, 11, 1], [i._node_id for i in self.dag.nodes_on_wire(qbit)]) + self.assertEqual([10, 11], [i._node_id for i in self.dag.nodes_on_wire(qbit, only_ops=True)]) cbit = self.dag.clbits()[0] - self.assertEqual([7, 8], [i._node_id for i in self.dag.nodes_on_wire(cbit)]) + self.assertEqual([6, 7], [i._node_id for i in self.dag.nodes_on_wire(cbit)]) self.assertEqual([], [i._node_id for i in self.dag.nodes_on_wire(cbit, only_ops=True)]) with self.assertRaises(DAGCircuitError): From 6b93c85f5a051589b6f8bce0ef03246e0b5ad393 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 6 Feb 2020 11:19:41 -0500 Subject: [PATCH 07/27] Return iterator for successors and predecessors The predecessors and successors methods of the DAGCircuit class were expecting an iterator returned not a list. Thist commit corrects the return type by wrapping the list in iter(). --- qiskit/dagcircuit/dagcircuit.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 331b170ca19c..5116098b43de 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -1001,16 +1001,16 @@ def longest_path(self): def successors(self, node): """Returns iterator of the successors of a node as DAGNodes.""" - return self._multi_graph.successors(node._node_id) + return iter(self._multi_graph.successors(node._node_id)) def predecessors(self, node): """Returns iterator of the predecessors of a node as DAGNodes.""" - return self._multi_graph.predecessors(node._node_id) + return iter(self._multi_graph.predecessors(node._node_id)) def quantum_predecessors(self, node): """Returns iterator of the predecessors of a node that are connected by a quantum edge as DAGNodes.""" - for predecessor in reversed(self.predecessors(node)): + for predecessor in reversed(list(self.predecessors(node))): if isinstance( self._multi_graph.get_edge_data( predecessor._node_id, node._node_id)['wire'], Qubit): @@ -1034,7 +1034,7 @@ def bfs_successors(self, node): def quantum_successors(self, node): """Returns iterator of the successors of a node that are connected by a quantum edge as DAGNodes.""" - for successor in reversed(self.successors(node)): + for successor in reversed(list(self.successors(node))): if isinstance(self._multi_graph.get_edge_data( node._node_id, successor._node_id)['wire'], Qubit): yield successor From 463f3117985350109fd468f3437fe9617fcaa734 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Sat, 8 Feb 2020 18:32:26 -0500 Subject: [PATCH 08/27] Update longest_path method The output of retworkx.dag_longest_path() returns the node indexes, not the nodes themselves. This commit updates the DAGCircuit.longest_path() method to make sure it's returning DAGNode objects, not the indexes. --- qiskit/dagcircuit/dagcircuit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 5116098b43de..13af7dcd458b 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -997,7 +997,7 @@ def threeQ_or_more_gates(self): def longest_path(self): """Returns the longest path in the dag as a list of DAGNodes.""" - return rx.dag_longest_path(self._multi_graph) + return [self._id_to_node[x] for x in rx.dag_longest_path(self._multi_graph)] def successors(self, node): """Returns iterator of the successors of a node as DAGNodes.""" From 9aaa48b1ea3b6556b7b08e4325f83a09dc667eee Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Sat, 8 Feb 2020 19:18:14 -0500 Subject: [PATCH 09/27] Update bfs_successors return type --- qiskit/dagcircuit/dagcircuit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 13af7dcd458b..6ae7b8231ab8 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -1029,7 +1029,7 @@ def bfs_successors(self, node): Returns an iterator of tuples of (DAGNode, [DAGNodes]) where the DAGNode is the current node and [DAGNode] is its successors in BFS order. """ - return rx.bfs_successors(self._multi_graph, node) + return iter(rx.bfs_successors(self._multi_graph, node._node_id)) def quantum_successors(self, node): """Returns iterator of the successors of a node that are From eab76f687c2d31f97dbffe674266f3aca5bc2440 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Sun, 9 Feb 2020 09:57:14 -0500 Subject: [PATCH 10/27] Use node_id for apply_operation_back_conditional_tests The apply operation back conditional tests were using in_edges and out_edges to check the structure of the dag. However with retworkx the return type from those functions change to return the node id instead of the node instance. This was causing the tests to fail because it was attempting to compare a DAGNode object with an int. This commit fixes the issue by making comparing DAGNode._node_id to the value of in_edges() and out_edges(). --- test/python/test_dagcircuit.py | 39 +++++++++++++++++----------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/test/python/test_dagcircuit.py b/test/python/test_dagcircuit.py index 6026af5a2ca8..7c2d492d2afb 100644 --- a/test/python/test_dagcircuit.py +++ b/test/python/test_dagcircuit.py @@ -220,22 +220,22 @@ def test_apply_operation_back_conditional(self): self.assertEqual( sorted(self.dag._multi_graph.in_edges(h_node._node_id)), sorted([ - (self.dag.input_map[self.qubit2], h_node, + (self.dag.input_map[self.qubit2]._node_id, h_node._node_id, {'wire': self.qubit2, 'name': 'qr[2]'}), - (self.dag.input_map[self.clbit0], h_node, + (self.dag.input_map[self.clbit0]._node_id, h_node._node_id, {'wire': self.clbit0, 'name': 'cr[0]'}), - (self.dag.input_map[self.clbit1], h_node, + (self.dag.input_map[self.clbit1]._node_id, h_node._node_id, {'wire': self.clbit1, 'name': 'cr[1]'}), ])) self.assertEqual( sorted(self.dag._multi_graph.out_edges(h_node._node_id)), sorted([ - (h_node, self.dag.output_map[self.qubit2], + (h_node._node_id, self.dag.output_map[self.qubit2]._node_id, {'wire': self.qubit2, 'name': 'qr[2]'}), - (h_node, self.dag.output_map[self.clbit0], + (h_node._node_id, self.dag.output_map[self.clbit0]._node_id, {'wire': self.clbit0, 'name': 'cr[0]'}), - (h_node, self.dag.output_map[self.clbit1], + (h_node._node_id, self.dag.output_map[self.clbit1]._node_id, {'wire': self.clbit1, 'name': 'cr[1]'}), ])) @@ -262,22 +262,22 @@ def test_apply_operation_back_conditional_measure(self): self.assertEqual( sorted(self.dag._multi_graph.in_edges(meas_node._node_id)), sorted([ - (self.dag.input_map[self.qubit0], meas_node, + (self.dag.input_map[self.qubit0]._node_id, meas_node._node_id, {'wire': self.qubit0, 'name': 'qr[0]'}), - (self.dag.input_map[self.clbit0], meas_node, + (self.dag.input_map[self.clbit0]._node_id, meas_node._node_id, {'wire': self.clbit0, 'name': 'cr[0]'}), - (self.dag.input_map[new_creg[0]], meas_node, + (self.dag.input_map[new_creg[0]]._node_id, meas_node._node_id, {'wire': Clbit(new_creg, 0), 'name': 'cr2[0]'}), ])) self.assertEqual( sorted(self.dag._multi_graph.out_edges(meas_node._node_id)), sorted([ - (meas_node, self.dag.output_map[self.qubit0], + (meas_node._node_id, self.dag.output_map[self.qubit0]._node_id, {'wire': self.qubit0, 'name': 'qr[0]'}), - (meas_node, self.dag.output_map[self.clbit0], + (meas_node._node_id, self.dag.output_map[self.clbit0]._node_id, {'wire': self.clbit0, 'name': 'cr[0]'}), - (meas_node, self.dag.output_map[new_creg[0]], + (meas_node._node_id, self.dag.output_map[new_creg[0]]._node_id, {'wire': Clbit(new_creg, 0), 'name': 'cr2[0]'}), ])) @@ -301,22 +301,22 @@ def test_apply_operation_back_conditional_measure_to_self(self): self.assertEqual( sorted(self.dag._multi_graph.in_edges(meas_node._node_id)), sorted([ - (self.dag.input_map[self.qubit1], meas_node, + (self.dag.input_map[self.qubit1]._node_id, meas_node._node_id, {'wire': self.qubit1, 'name': 'qr[1]'}), - (self.dag.input_map[self.clbit0], meas_node, + (self.dag.input_map[self.clbit0]._node_id, meas_node._node_id, {'wire': self.clbit0, 'name': 'cr[0]'}), - (self.dag.input_map[self.clbit1], meas_node, + (self.dag.input_map[self.clbit1]._node_id, meas_node._node_id, {'wire': self.clbit1, 'name': 'cr[1]'}), ])) self.assertEqual( - sorted(self.dag._multi_graph.out_edges(meas_node)), + sorted(self.dag._multi_graph.out_edges(meas_node._node_id)), sorted([ - (meas_node, self.dag.output_map[self.qubit1], + (meas_node._node_id, self.dag.output_map[self.qubit1]._node_id, {'wire': self.qubit1, 'name': 'qr[1]'}), - (meas_node, self.dag.output_map[self.clbit0], + (meas_node._node_id, self.dag.output_map[self.clbit0]._node_id, {'wire': self.clbit0, 'name': 'cr[0]'}), - (meas_node, self.dag.output_map[self.clbit1], + (meas_node._node_id, self.dag.output_map[self.clbit1]._node_id, {'wire': self.clbit1, 'name': 'cr[1]'}), ])) @@ -452,7 +452,6 @@ def test_get_named_nodes(self): def test_topological_nodes(self): """The topological_nodes() method""" - self.maxDiff = None self.dag.apply_operation_back(CnotGate(), [self.qubit0, self.qubit1], []) self.dag.apply_operation_back(HGate(), [self.qubit0], []) self.dag.apply_operation_back(CnotGate(), [self.qubit2, self.qubit1], []) From 6fb58dc775e4e23565f08ebed789b088874241b6 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Sun, 9 Feb 2020 18:25:30 -0500 Subject: [PATCH 11/27] Fix layers methods --- qiskit/dagcircuit/dagcircuit.py | 2 +- test/python/test_dagcircuit.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 6ae7b8231ab8..1ce7eb9182cc 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -1194,7 +1194,7 @@ def multigraph_layers(self): while cur_layer: for node in cur_layer: # Count multiedges with multiplicity. - for successor in self._multi_graph.successors(node._node_id): + for successor in set(self._multi_graph.successors(node._node_id)): multiplicity = len(self._multi_graph.get_all_edge_data( node._node_id, successor._node_id)) if successor in predecessor_count: diff --git a/test/python/test_dagcircuit.py b/test/python/test_dagcircuit.py index 7c2d492d2afb..f74ce695c6b1 100644 --- a/test/python/test_dagcircuit.py +++ b/test/python/test_dagcircuit.py @@ -671,7 +671,7 @@ def test_layers_maintains_order(self): qr = QuantumRegister(1, 'q0') # the order the nodes should be in - truth = [('in', 'q0[0]', 1), ('op', 'x', 3), ('op', 'id', 4), ('out', 'q0[0]', 2)] + truth = [('in', 'q0[0]', 0), ('op', 'x', 2), ('op', 'id', 3), ('out', 'q0[0]', 1)] # this only occurred sometimes so has to be run more than once # (10 times seemed to always be enough for this bug to show at least once) From ab94ea142c7af3a392cf22adf6b6b94ed4026df0 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Mon, 10 Feb 2020 05:57:07 -0500 Subject: [PATCH 12/27] De-duplicate in retworkx not dagcircuit Using python to deduplicate the output from retworkx in the dagcircuit class gets increasinly inefficient as the number of nodes increases. Relying on retworkx to deduplicate moving forward will be more efficient. --- qiskit/dagcircuit/dagcircuit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 1ce7eb9182cc..6ae7b8231ab8 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -1194,7 +1194,7 @@ def multigraph_layers(self): while cur_layer: for node in cur_layer: # Count multiedges with multiplicity. - for successor in set(self._multi_graph.successors(node._node_id)): + for successor in self._multi_graph.successors(node._node_id): multiplicity = len(self._multi_graph.get_all_edge_data( node._node_id, successor._node_id)) if successor in predecessor_count: From 2f501c5ddf1708a930c136bcc1e4430138ffd01b Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Mon, 10 Feb 2020 13:57:09 -0500 Subject: [PATCH 13/27] Fix missing node id from has_edge call --- qiskit/transpiler/passes/utils/merge_adjacent_barriers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qiskit/transpiler/passes/utils/merge_adjacent_barriers.py b/qiskit/transpiler/passes/utils/merge_adjacent_barriers.py index 86e6f98231aa..87468fee33b6 100644 --- a/qiskit/transpiler/passes/utils/merge_adjacent_barriers.py +++ b/qiskit/transpiler/passes/utils/merge_adjacent_barriers.py @@ -113,7 +113,7 @@ def _collect_potential_merges(dag, barriers): for next_barrier in barriers[1:]: # Ensure barriers are adjacent before checking if they are mergeable. - if dag._multi_graph.has_edge(end_of_barrier, next_barrier): + if dag._multi_graph.has_edge(end_of_barrier._node_id, next_barrier._node_id): # Remove all barriers that have already been included in this new barrier from the # set of ancestors/descendants as they will be removed from the new DAG when it is From 20004e833b80732bbd0a903e42dc40ae9c4cc998 Mon Sep 17 00:00:00 2001 From: Lauren Capelluto Date: Mon, 10 Feb 2020 15:30:16 -0500 Subject: [PATCH 14/27] The networkx node indices were one-indexed. retworkx is zero-indexed. Update hardcoded test cases to be zero indexed to match retworkx --- .../transpiler/test_commutation_analysis.py | 60 +++++++++---------- test/python/transpiler/test_pass_scheduler.py | 2 +- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/test/python/transpiler/test_commutation_analysis.py b/test/python/transpiler/test_commutation_analysis.py index 8b290e1310f8..7c9675197033 100644 --- a/test/python/transpiler/test_commutation_analysis.py +++ b/test/python/transpiler/test_commutation_analysis.py @@ -86,18 +86,18 @@ def test_all_gates(self): self.pass_.run(dag) - expected = {'qr[0]': [[1], + expected = {'qr[0]': [[0], + [4], [5], [6], - [7], - [8, 9, 10, 11], + [7, 8, 9, 10], + [11], [12], [13], [14], [15], - [16], - [2]], - 'qr[1]': [[3], [14], [15], [16], [4]]} + [1]], + 'qr[1]': [[2], [13], [14], [15], [3]]} self.assertCommutationSet(self.pset["commutation_set"], expected) def test_non_commutative_circuit(self): @@ -116,7 +116,7 @@ def test_non_commutative_circuit(self): self.pass_.run(dag) - expected = {'qr[0]': [[1], [7], [2]], 'qr[1]': [[3], [8], [4]], 'qr[2]': [[5], [9], [6]]} + expected = {'qr[0]': [[0], [6], [1]], 'qr[1]': [[2], [7], [3]], 'qr[2]': [[4], [8], [5]]} self.assertCommutationSet(self.pset["commutation_set"], expected) def test_non_commutative_circuit_2(self): @@ -137,9 +137,9 @@ def test_non_commutative_circuit_2(self): self.pass_.run(dag) - expected = {'qr[0]': [[1], [7], [2]], - 'qr[1]': [[3], [7], [9], [4]], - 'qr[2]': [[5], [8], [9], [6]]} + expected = {'qr[0]': [[0], [6], [1]], + 'qr[1]': [[2], [6], [8], [3]], + 'qr[2]': [[4], [7], [8], [5]]} self.assertCommutationSet(self.pset["commutation_set"], expected) def test_commutative_circuit(self): @@ -161,9 +161,9 @@ def test_commutative_circuit(self): self.pass_.run(dag) - expected = {'qr[0]': [[1], [7], [2]], - 'qr[1]': [[3], [7, 9], [4]], - 'qr[2]': [[5], [8], [9], [6]]} + expected = {'qr[0]': [[0], [6], [1]], + 'qr[1]': [[2], [6, 8], [3]], + 'qr[2]': [[4], [7], [8], [5]]} self.assertCommutationSet(self.pset["commutation_set"], expected) def test_commutative_circuit_2(self): @@ -187,9 +187,9 @@ def test_commutative_circuit_2(self): self.pass_.run(dag) - expected = {'qr[0]': [[1], [7, 8], [2]], - 'qr[1]': [[3], [7, 10], [4]], - 'qr[2]': [[5], [9], [10], [6]]} + expected = {'qr[0]': [[0], [6, 7], [1]], + 'qr[1]': [[2], [6, 9], [3]], + 'qr[2]': [[4], [8], [9], [5]]} self.assertCommutationSet(self.pset["commutation_set"], expected) def test_commutative_circuit_3(self): @@ -215,9 +215,9 @@ def test_commutative_circuit_3(self): self.pass_.run(dag) - expected = {'qr[0]': [[1], [7, 9, 11, 13], [2]], - 'qr[1]': [[3], [7, 10, 11], [14], [4]], - 'qr[2]': [[5], [8], [10], [12, 14], [6]]} + expected = {'qr[0]': [[0], [6, 8, 10, 12], [1]], + 'qr[1]': [[2], [6, 9, 10], [13], [3]], + 'qr[2]': [[4], [7], [9], [11, 13], [5]]} self.assertCommutationSet(self.pset["commutation_set"], expected) def test_jordan_wigner_type_circuit(self): @@ -253,12 +253,12 @@ def test_jordan_wigner_type_circuit(self): self.pass_.run(dag) - expected = {'qr[0]': [[1], [13, 23], [2]], - 'qr[1]': [[3], [13], [14, 22], [23], [4]], - 'qr[2]': [[5], [14], [15, 21], [22], [6]], - 'qr[3]': [[7], [15], [16, 20], [21], [8]], - 'qr[4]': [[9], [16], [17, 19], [20], [10]], - 'qr[5]': [[11], [17], [18], [19], [12]]} + expected = {'qr[0]': [[0], [12, 22], [1]], + 'qr[1]': [[2], [12], [13, 21], [22], [3]], + 'qr[2]': [[4], [13], [14, 20], [21], [5]], + 'qr[3]': [[6], [14], [15, 19], [20], [7]], + 'qr[4]': [[8], [15], [16, 18], [19], [9]], + 'qr[5]': [[10], [16], [17], [18], [11]]} self.assertCommutationSet(self.pset["commutation_set"], expected) def test_all_commute_circuit(self): @@ -279,11 +279,11 @@ def test_all_commute_circuit(self): self.pass_.run(dag) - expected = {'qr[0]': [[1], [11, 15, 17], [2]], - 'qr[1]': [[3], [11, 12, 17, 18], [4]], - 'qr[2]': [[5], [12, 14, 18, 20], [6]], - 'qr[3]': [[7], [13, 14, 19, 20], [8]], - 'qr[4]': [[9], [13, 16, 19], [10]]} + expected = {'qr[0]': [[0], [10, 14, 16], [1]], + 'qr[1]': [[2], [10, 11, 16, 17], [3]], + 'qr[2]': [[4], [11, 13, 17, 19], [5]], + 'qr[3]': [[6], [12, 13, 18, 19], [7]], + 'qr[4]': [[8], [12, 15, 18], [9]]} self.assertCommutationSet(self.pset["commutation_set"], expected) diff --git a/test/python/transpiler/test_pass_scheduler.py b/test/python/transpiler/test_pass_scheduler.py index de7f8f4090cb..69f30f6d8b9f 100644 --- a/test/python/transpiler/test_pass_scheduler.py +++ b/test/python/transpiler/test_pass_scheduler.py @@ -296,7 +296,7 @@ def test_fenced_dag(self): self.passmanager.append(PassI_Bad_AP()) self.assertSchedulerRaises(circ, self.passmanager, ['run analysis pass PassI_Bad_AP', - 'cx_runs: {(5, 6, 7, 8)}'], + 'cx_runs: {(4, 5, 6, 7)}'], TranspilerError) def test_analysis_pass_is_idempotent(self): From 3caab84779707da0ce42ac6d44cb20f9cd0fb13f Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Mon, 10 Feb 2020 15:56:56 -0500 Subject: [PATCH 15/27] Bump minimum retworkx version to working version --- requirements.txt | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index b58707c38c4e..4e9d103ae843 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ marshmallow>=3,<4 marshmallow_polyfield>=5.7,<6 networkx>=2.2;python_version>'3.5' networkx>=2.2,<2.4;python_version=='3.5' -retworkx>=0.1.0 +retworkx>=0.2.0 numpy>=1.13 ply>=3.10 psutil>=5 diff --git a/setup.py b/setup.py index 007c8efc4bc2..ea92b23eb107 100755 --- a/setup.py +++ b/setup.py @@ -31,7 +31,7 @@ "networkx>=2.2;python_version>'3.5'", # Networkx 2.4 is the final version with python 3.5 support. "networkx>=2.2,<2.4;python_version=='3.5'", - "retworkx>=0.1.0", + "retworkx>=0.2.0", "numpy>=1.13", "ply>=3.10", "psutil>=5", From a7e16fc2ddfd234ea5fd042b0fb58276944967c5 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Tue, 11 Feb 2020 06:37:42 -0500 Subject: [PATCH 16/27] Fix lint --- qiskit/dagcircuit/dagcircuit.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 6ae7b8231ab8..09d8480a9810 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -156,11 +156,10 @@ def _add_wire(self, wire): self._id_to_node[output_map_wire] = outp_node self.input_map[wire] = inp_node self.output_map[wire] = outp_node - - self._multi_graph.add_edge(inp_node._node_id, outp_node._node_id, - {'name': "%s[%s]" % (wire.register.name, wire.index), + {'name': "%s[%s]" % (wire.register.name, + wire.index), 'wire': wire}) else: raise DAGCircuitError("duplicate wire %s" % (wire,)) From 94cf754d16174aa53cacb13fa3676cbf62f70aee Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Tue, 11 Feb 2020 07:02:32 -0500 Subject: [PATCH 17/27] Fix lint again --- .pylintrc | 2 +- qiskit/dagcircuit/dagcircuit.py | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.pylintrc b/.pylintrc index a0fabbeafb60..fa66eb0e32f3 100644 --- a/.pylintrc +++ b/.pylintrc @@ -298,7 +298,7 @@ ignore-mixin-members=yes # (useful for modules/projects where namespaces are manipulated during runtime # and thus existing member attributes cannot be deduced by static analysis. It # supports qualified module names, as well as Unix pattern matching. -ignored-modules=matplotlib.cm,numpy.random +ignored-modules=matplotlib.cm,numpy.random,retworkx # List of class names for which member attributes should not be checked (useful # for classes with dynamically set attributes). This supports the use of diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 09d8480a9810..733c41540996 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -12,6 +12,8 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. +# pylint: disable=unnecessary-lambda + """ Object to represent a quantum circuit as a directed acyclic graph (DAG). @@ -215,6 +217,8 @@ def _add_op_node(self, op, qargs, cargs, condition=None): qargs (list[Qubit]): list of quantum wires to attach to. cargs (list[Clbit]): list of classical wires to attach to. condition (tuple or None): optional condition (ClassicalRegister, int) + Returns: + int: The integer node index for the new op node on the DAG """ node_properties = { "type": "op", @@ -1277,10 +1281,11 @@ def nodes_on_wire(self, wire, only_ops=False): # TODO(mtreinish): Add function in retworkx that does this nested api for node_index in self._multi_graph.adj(current_node._node_id): node = self._id_to_node[node_index] - try: + if self._multi_graph.has_edge(current_node._node_id, + node_index): edge_data = self._multi_graph.get_all_edge_data( current_node._node_id, node_index) - except Exception: + else: edge_data = self._multi_graph.get_all_edge_data( node_index, current_node._node_id) if any(wire == edge['wire'] for edge in edge_data): From c966f02402e835021c20ae728c82c9e16c5ccfb4 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 13 Feb 2020 21:58:56 -0500 Subject: [PATCH 18/27] Rename variable in _add_wire() Co-authored-by: Kevin Krsulich --- qiskit/dagcircuit/dagcircuit.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 733c41540996..484806b82875 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -150,12 +150,12 @@ def _add_wire(self, wire): inp_node = DAGNode(data_dict={'type': 'in', 'name': wire_name, 'wire': wire}) outp_node = DAGNode(data_dict={'type': 'out', 'name': wire_name, 'wire': wire}) - input_map_wire = self._multi_graph.add_node(inp_node) - output_map_wire = self._multi_graph.add_node(outp_node) - inp_node._node_id = input_map_wire - outp_node._node_id = output_map_wire - self._id_to_node[input_map_wire] = inp_node - self._id_to_node[output_map_wire] = outp_node + input_map_id = self._multi_graph.add_node(inp_node) + output_map_id = self._multi_graph.add_node(outp_node) + inp_node._node_id = input_map_id + outp_node._node_id = output_map_id + self._id_to_node[input_map_id] = inp_node + self._id_to_node[output_map_id] = outp_node self.input_map[wire] = inp_node self.output_map[wire] = outp_node self._multi_graph.add_edge(inp_node._node_id, From df5716141e879e74398aac6f46ee5b63d44e1bea Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 13 Feb 2020 22:06:11 -0500 Subject: [PATCH 19/27] Fix multigraph bug in quantum_* dagcircuit methods There was a potential bug in the quantum_successors and quantum_predecessors methods in the case of a multigraph dag that had a classical wire and a quantum wire for the same node. retworkx.PyDAG.get_edge() returns a single node and if there is more than one, as in a multigraph we'd end up in a case where we'd accidently exclude a node in the output because we fetched the incorrect edge from the PyDAG class. --- qiskit/dagcircuit/dagcircuit.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 484806b82875..c18c6b1ab8d1 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -1014,9 +1014,9 @@ def quantum_predecessors(self, node): """Returns iterator of the predecessors of a node that are connected by a quantum edge as DAGNodes.""" for predecessor in reversed(list(self.predecessors(node))): - if isinstance( - self._multi_graph.get_edge_data( - predecessor._node_id, node._node_id)['wire'], Qubit): + if any(isinstance(x['wire'], Qubit) for x in + self._multi_graph.get_all_edge_data( + predecessor._node_id, node._node_id)): yield predecessor def ancestors(self, node): @@ -1038,8 +1038,9 @@ def quantum_successors(self, node): """Returns iterator of the successors of a node that are connected by a quantum edge as DAGNodes.""" for successor in reversed(list(self.successors(node))): - if isinstance(self._multi_graph.get_edge_data( - node._node_id, successor._node_id)['wire'], Qubit): + if any(isinstance(x['wire'], Qubit) for x in + self._multi_graph.get_all_edge_data( + node._node_id, successor._node_id)): yield successor def remove_op_node(self, node): From 82c27cd43c7352986601a6c422b64f4262b735db Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 13 Feb 2020 22:34:38 -0500 Subject: [PATCH 20/27] Update deepcopy comment in __eq__ --- qiskit/dagcircuit/dagcircuit.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index c18c6b1ab8d1..3cb0ef1fc0c1 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -702,7 +702,8 @@ def _full_pred_succ_maps(self, pred_map, succ_map, input_circuit, return full_pred_map, full_succ_map def __eq__(self, other): - # TODO this works but is a horrible way to do this + # TODO remove deepcopy calls after + # https://github.com/mtreinish/retworkx/issues/27 is fixed slf = copy.deepcopy(self._multi_graph) oth = copy.deepcopy(other._multi_graph) return rx.is_isomorphic_node_match(slf, oth, From c9252871b6e37500056d36e78c850df8cc94e651 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 13 Feb 2020 22:36:19 -0500 Subject: [PATCH 21/27] Fix lint --- qiskit/dagcircuit/dagcircuit.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 3cb0ef1fc0c1..b7043e77cd05 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -1016,7 +1016,7 @@ def quantum_predecessors(self, node): connected by a quantum edge as DAGNodes.""" for predecessor in reversed(list(self.predecessors(node))): if any(isinstance(x['wire'], Qubit) for x in - self._multi_graph.get_all_edge_data( + self._multi_graph.get_all_edge_data( predecessor._node_id, node._node_id)): yield predecessor @@ -1041,7 +1041,7 @@ def quantum_successors(self, node): for successor in reversed(list(self.successors(node))): if any(isinstance(x['wire'], Qubit) for x in self._multi_graph.get_all_edge_data( - node._node_id, successor._node_id)): + node._node_id, successor._node_id)): yield successor def remove_op_node(self, node): From 4620c4d44446cf2b414aa31ca01cb479f3ee9880 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 13 Feb 2020 22:37:59 -0500 Subject: [PATCH 22/27] Fix lint again --- qiskit/dagcircuit/dagcircuit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index b7043e77cd05..ea58a98681b5 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -1017,7 +1017,7 @@ def quantum_predecessors(self, node): for predecessor in reversed(list(self.predecessors(node))): if any(isinstance(x['wire'], Qubit) for x in self._multi_graph.get_all_edge_data( - predecessor._node_id, node._node_id)): + predecessor._node_id, node._node_id)): yield predecessor def ancestors(self, node): From 438875c616482dcea6133e4643af1be42944b713 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 20 Feb 2020 15:58:31 -0500 Subject: [PATCH 23/27] Use retwork for multigraph_layers --- qiskit/dagcircuit/dagcircuit.py | 25 ++----------------------- requirements.txt | 2 +- setup.py | 2 +- 3 files changed, 4 insertions(+), 25 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index ea58a98681b5..b39d2ca56392 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -1192,29 +1192,8 @@ def serial_layers(self): def multigraph_layers(self): """Yield layers of the multigraph.""" - predecessor_count = dict() # Dict[node, predecessors not visited] - cur_layer = self.input_map.values() - yield cur_layer - next_layer = [] - while cur_layer: - for node in cur_layer: - # Count multiedges with multiplicity. - for successor in self._multi_graph.successors(node._node_id): - multiplicity = len(self._multi_graph.get_all_edge_data( - node._node_id, successor._node_id)) - if successor in predecessor_count: - predecessor_count[successor] -= multiplicity - else: - predecessor_count[successor] = \ - self._multi_graph.in_degree(successor._node_id) - multiplicity - - if predecessor_count[successor] == 0: - next_layer.append(successor) - del predecessor_count[successor] - - yield next_layer - cur_layer = next_layer - next_layer = [] + first_layer = [x._node_id for x in self.input_map.values()] + return iter(rx.layers(self._multi_graph, first_layer)) def collect_runs(self, namelist): """Return a set of non-conditional runs of "op" nodes with the given names. diff --git a/requirements.txt b/requirements.txt index 4e9d103ae843..43f1949f9dc2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ marshmallow>=3,<4 marshmallow_polyfield>=5.7,<6 networkx>=2.2;python_version>'3.5' networkx>=2.2,<2.4;python_version=='3.5' -retworkx>=0.2.0 +retworkx>=0.3.0 numpy>=1.13 ply>=3.10 psutil>=5 diff --git a/setup.py b/setup.py index a9877071c974..599c0ee19e59 100755 --- a/setup.py +++ b/setup.py @@ -31,7 +31,7 @@ "networkx>=2.2;python_version>'3.5'", # Networkx 2.4 is the final version with python 3.5 support. "networkx>=2.2,<2.4;python_version=='3.5'", - "retworkx>=0.2.0", + "retworkx>=0.3.0", "numpy>=1.13", "ply>=3.10", "psutil>=5", From 5f46cc5a21c58be28a71b002b66a49f62d21dbda Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Mon, 24 Feb 2020 13:34:26 -0500 Subject: [PATCH 24/27] Switch edges to use out_edges instead of in_edges As @itoko pointed out in review the edges() function in networkx was using the outgoing edges from the graph not the incoming edges for it's edges() function. This doesn't normally matter for the case of returning edges for the entire dagcircuit because the list will be the same given the structure. But, there was a possible edge case when edges() was called with an output node or input node that would have behaved differently. This was not caught in the tests because there are no places in the code that currently do this, but there are potentially users of the function relying on this behavior currently so it's better to mirror the functionality. To ensure this remains consistent a test case is added to verify input and output nodes behave as expected when passed into edges(). --- qiskit/dagcircuit/dagcircuit.py | 14 ++++++++++++-- test/python/test_dagcircuit.py | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index b39d2ca56392..6d74c7f31c80 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -935,8 +935,17 @@ def nodes(self): def edges(self, nodes=None): """Iterator for edge values and source and dest node + This works by returning the output edges from the specified nodes. If + no nodes are specified all edges from the graph are returned. + + Args: + nodes(DAGNode|list(DAGNode): Either a list of nodes or a single + input node. If none is specified all edges are returned from + the graph. + Yield: - edge: the edge. + edge: the edge in the same format as out_edges the tuple + (source node, destination node, edge data) """ if nodes is None: nodes = self._multi_graph.nodes() @@ -944,7 +953,8 @@ def edges(self, nodes=None): elif isinstance(nodes, DAGNode): nodes = [nodes] for node in nodes: - for source, dest, edge in self._multi_graph.in_edges(node._node_id): + raw_nodes = self._multi_graph.out_edges(node._node_id) + for source, dest, edge in raw_nodes: yield self._id_to_node[source], self._id_to_node[dest], edge def op_nodes(self, op=None): diff --git a/test/python/test_dagcircuit.py b/test/python/test_dagcircuit.py index df5c4c4dbed0..7fe9e5cbc02a 100644 --- a/test/python/test_dagcircuit.py +++ b/test/python/test_dagcircuit.py @@ -203,6 +203,20 @@ def test_apply_operation_back(self): self.assertEqual(len(list(self.dag.nodes())), 16) self.assertEqual(len(list(self.dag.edges())), 17) + def test_edges(self): + """Test that DAGCircuit.edges() behaves as expected with ops.""" + self.dag.apply_operation_back(HGate(), [self.qubit0], [], condition=None) + self.dag.apply_operation_back(CXGate(), [self.qubit0, self.qubit1], [], condition=None) + self.dag.apply_operation_back(Measure(), [self.qubit1, self.clbit1], [], condition=None) + self.dag.apply_operation_back(XGate(), [self.qubit1], [], condition=self.condition) + self.dag.apply_operation_back(Measure(), [self.qubit0, self.clbit0], [], condition=None) + self.dag.apply_operation_back(Measure(), [self.qubit1, self.clbit1], [], condition=None) + out_edges = self.dag.edges(self.dag.output_map.values()) + self.assertEqual(list(out_edges), []) + in_edges = self.dag.edges(self.dag.input_map.values()) + # number of edges for input nodes should be the same as number of wires + self.assertEqual(len(list(in_edges)), 5) + def test_apply_operation_back_conditional(self): """Test consistency of apply_operation_back with condition set.""" From af1905120381afdee3597bebe09769a1ab51d334 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Mon, 24 Feb 2020 13:50:47 -0500 Subject: [PATCH 25/27] Remove unecessary iter() from tests There was as stary iter() wrapper around the output of topological_nodes() in the tests. This was leftover from an earlier iteration of the branch where topological_nodes() returned a list. This has been fixed (to maintain compatibility with the current api) so the iter is no longer needed. --- test/python/test_dagcircuit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/python/test_dagcircuit.py b/test/python/test_dagcircuit.py index 7fe9e5cbc02a..7ff69da37bcc 100644 --- a/test/python/test_dagcircuit.py +++ b/test/python/test_dagcircuit.py @@ -579,7 +579,7 @@ def test_remove_non_op_node(self): """Try to remove a non-op node with remove_op_node method.""" self.dag.apply_operation_back(HGate(), [self.qubit0]) - in_node = next(iter(self.dag.topological_nodes())) + in_node = next(self.dag.topological_nodes()) self.assertRaises(DAGCircuitError, self.dag.remove_op_node, in_node) def test_dag_collect_runs(self): From c67dc50fd9121d3384842769dc3a50dffee33e66 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Mon, 24 Feb 2020 14:18:10 -0500 Subject: [PATCH 26/27] Remove unecessary list() casts --- qiskit/dagcircuit/dagcircuit.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 6d74c7f31c80..411ab8d28ac5 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -269,7 +269,7 @@ def apply_operation_back(self, op, qargs=None, cargs=None, condition=None): # and adding new edges from the operation node to each output node al = [qargs, all_cbits] for q in itertools.chain(*al): - ie = list(self._multi_graph.predecessors(self.output_map[q]._node_id)) + ie = self._multi_graph.predecessors(self.output_map[q]._node_id) if len(ie) != 1: raise DAGCircuitError("output node has multiple in-edges") @@ -312,7 +312,7 @@ def apply_operation_front(self, op, qargs, cargs, condition=None): # and adding new edges to the operation node from each input node al = [qargs, all_cbits] for q in itertools.chain(*al): - ie = list(self._multi_graph.successors(self.input_map[q]._node_id)) + ie = self._multi_graph.successors(self.input_map[q]._node_id) if len(ie) != 1: raise DAGCircuitError("input node has multiple out-edges") self._multi_graph.add_edge(node_index, ie[0]._node_id, @@ -695,7 +695,7 @@ def _full_pred_succ_maps(self, pred_map, succ_map, input_circuit, full_succ_map[w] = self.output_map[w] full_pred_map[w] = self._multi_graph.predecessors( self.output_map[w])[0] - if len(list(self._multi_graph.predecessors(self.output_map[w]))) != 1: + if len(self._multi_graph.predecessors(self.output_map[w])) != 1: raise DAGCircuitError("too many predecessors for %s[%d] " "output node" % (w.register, w.index)) @@ -841,7 +841,7 @@ def substitute_node_with_dag(self, node, input_dag, wires=None): full_succ_map[w], dict(name="%s[%s]" % (w.register.name, w.index), wire=w)) - o_pred = list(self._multi_graph.predecessors(self.output_map[w]._node_id)) + o_pred = self._multi_graph.predecessors(self.output_map[w]._node_id) if len(o_pred) > 1: if len(o_pred) != 2: raise DAGCircuitError("expected 2 predecessors here") @@ -1236,7 +1236,7 @@ def collect_runs(self, namelist): s[0].condition is None: group.append(s[0]) nodes_seen[s[0]] = True - s = list(self._multi_graph.successors(s[0]._node_id)) + s = self._multi_graph.successors(s[0]._node_id) if len(group) >= 1: group_list.append(tuple(group)) return set(group_list) From 58b1ad54191cc8ef49803a37ddc3f734a5d495ea Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Mon, 24 Feb 2020 14:47:29 -0500 Subject: [PATCH 27/27] Add release note --- releasenotes/notes/retworkx-85e619ebdb94132d.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 releasenotes/notes/retworkx-85e619ebdb94132d.yaml diff --git a/releasenotes/notes/retworkx-85e619ebdb94132d.yaml b/releasenotes/notes/retworkx-85e619ebdb94132d.yaml new file mode 100644 index 000000000000..87b77859e996 --- /dev/null +++ b/releasenotes/notes/retworkx-85e619ebdb94132d.yaml @@ -0,0 +1,11 @@ +--- +upgrade: + - | + A new requirement has been added to the requirements list, + `retworkx `_. It is an Apache 2.0 + licensed graph library that has a similar API to networkx and is being used + to significantly speed up the :class:`qiskit.dagcircuit.DAGCircuit` + operations as part of the transpiler. There are binaries published for all + the platforms supported by Qiskit Terra but if you're using a platform + where there aren't precompiled binaries published refer to the retworkx + documentation for instructions on pip installing from sdist.