Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up DAGCircuit.reverse_ops() #10448

Closed
wants to merge 3 commits into from
Closed

Speed up DAGCircuit.reverse_ops() #10448

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
df28edb
Speed up DAGCircuit.reverse_ops()
mtreinish Jul 19, 2023
a822a3d
Merge branch 'main' into reverse
mtreinish Jul 19, 2023
2d3e599
Merge branch 'main' into reverse
mtreinish Sep 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions qiskit/dagcircuit/dagcircuit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -838,17 +838,18 @@ def compose(self, other, qubits=None, clbits=None, front=False, inplace=True):
def reverse_ops(self):
"""Reverse the operations in the ``self`` circuit.

.. note::

The output reversed :class:`~.DAGCircuit` object will have shared
references for all operations and qubits to this dag.
Comment on lines +853 to +854
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qubits are immutable, so it's probably not worth mentioning them. If we're worried about ownership here, it could be worth adding a copy_operations flag like we've got for circuit_to_dag and dag_to_circuit, and replace the copy.copy with a copy.deepcopy based on that. I'm not certain if it's worth it, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My first attempt at this locally was using a deepcopy instead. The deepcopy on its own ended up being slower than the previous implementation of the method. I figured this was an ok tradeoff because the ownership here wasn't clearly established before. But I don't feel super strongly about it either.


Returns:
DAGCircuit: the reversed dag.
"""
# TODO: speed up
# pylint: disable=cyclic-import
from qiskit.converters import dag_to_circuit, circuit_to_dag

qc = dag_to_circuit(self)
reversed_qc = qc.reverse_ops()
reversed_dag = circuit_to_dag(reversed_qc)
return reversed_dag
out_graph = copy.copy(self)
out_graph._multi_graph = out_graph._multi_graph.copy()
out_graph._multi_graph.reverse()
return out_graph

def idle_wires(self, ignore=None):
"""Return idle wires.
Expand Down