add function to swap connected nodes in DAGCircuit #9160
Conversation
|
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
There was a problem hiding this comment.
Do you have a particular case / transpiler pass in mind for when you want this? There's a general hope (cc: @alexanderivrii) that we might be able to use DAGDependency as our principle driver type for dealing with commutation relations, though that's likely to be a long way off. If this is useful in the near-term, it generally seems like a useful helper.
Don't forget the release note.
qiskit/dagcircuit/dagcircuit.py
Outdated
| edge_parent = self._multi_graph.find_predecessors_by_edge(node1_id, edge_find)[0] | ||
| self._multi_graph.remove_edge(edge_parent._node_id, node1_id) | ||
| self._multi_graph.add_edge(edge_parent._node_id, node2_id, edge) | ||
| edge_child = self._multi_graph.find_successors_by_edge(node2_id, lambda x: x == edge)[0] |
There was a problem hiding this comment.
I don't know if there's an equivalent for predecessors, but this method could be find_adjacent_node_by_edge to avoid calculating the whole list.
There was a problem hiding this comment.
How do you make find_adjacent_node_by_edge filter on predecessor or successor?
There was a problem hiding this comment.
It's a little buried in the documentation, but it only finds successors - that's why I wasn't sure if there's an equivalent for the predecessors.
There was a problem hiding this comment.
Predecessors doesn't exist but there is a PR to add it,
Qiskit/rustworkx#756
Pull Request Test Coverage Report for Build 3950153453
💛 - Coveralls |
qiskit/dagcircuit/dagcircuit.py
Outdated
| def draw(self, scale=0.7, filename=None, style="color"): | ||
| def draw(self, scale=0.7, filename=None, style="color", **kwargs): |
There was a problem hiding this comment.
Can we shift this to a different PR? It seems unrelated.
There was a problem hiding this comment.
removed in ff987f1
The multiplexor partial swap wasn't passing. Not sure this is rerlated to this pr.
Co-authored-by: Jake Lishman <jake@binhbar.com>
kdk
added
Changelog: New Feature
* add function to swap connected nodes in DAGCircuit * linting. remove multiplexor test. The multiplexor partial swap wasn't passing. Not sure this is rerlated to this pr. * Update qiskit/dagcircuit/dagcircuit.py Co-authored-by: Jake Lishman <jake@binhbar.com> * build dagcircuit directly in tests * add spectator ops to swap tests * add release notes * remove dag visualization changes from this pr --------- Co-authored-by: Jake Lishman <jake@binhbar.com>
Summary
Sometimes when doing circuit optimization it is useful to be able to swap two connected gates for instance when they commute. If the two gates are on the same qubits this can be done with the existing method
substitute_node, however when the qubits are disjoint this can't be used. This pr adds a methodswap_nodeswhich allows this operation.Details and comments