Add dynamical decoupling transpiler pass #6619
Conversation
mtreinish
added
Changelog: New Feature
| if len(dag.qregs) != 1 or dag.qregs.get('q', None) is None: | ||
| raise TranspilerError('DD runs on physical circuits only') | ||
|
|
||
| new_dag = DAGCircuit() |
There was a problem hiding this comment.
This is probably fine, I'm just wondering why you went for doing a fresh dag vs just doing in place replacement with subtitute_node_with_dag for the dd sequence? With the in place approach you could probably just loop over dag.named_nodes() and work directly with only the delays. Did you benchmark to see which is faster?
There was a problem hiding this comment.
I haven't benchmarked this, agree it would be good to know. Probably as a follow up.
ajavadia
force-pushed
the
dynamical-decoupling-pass
branch
from
ea8a10f to
11e0380
Compare
mtreinish
changed the title
| dd_sequence = [] | ||
| dd_sequence_duration = 0 | ||
| for g in self._dd_sequence: | ||
| g.duration = self._durations.get(g, physical_qubit) | ||
| dd_sequence.append(g) | ||
| dd_sequence_duration += g.duration |
There was a problem hiding this comment.
Could be memoized for each qubit
There was a problem hiding this comment.
unfortunately the physical_qubit changes the duration. but I did realize that I needlessly created a dd_sequence again which is the same as self._dd_sequence
| else: | ||
| new_dag.apply_operation_back(nd.op, nd.qargs, nd.cargs) | ||
|
|
||
| new_dag.duration = dag.duration |
There was a problem hiding this comment.
Should these be part of dag._copy_circuit_metadata()?
There was a problem hiding this comment.
Potentially, though I think duration as metadata is a bit dangerous because it can be overwritten easily. In reality duration should be calculated, similar to how we don't attach depth to the circuit. But for now I'll add it, requires a bit of though to improve.
ajavadia
force-pushed
the
dynamical-decoupling-pass
branch
from
c71a62e to
e40d3f4
Compare
ajavadia
force-pushed
the
dynamical-decoupling-pass
branch
from
1d917c5 to
37fedeb
Compare
| dd_sequence_duration = 0 | ||
| for gate in self._dd_sequence: | ||
| gate.duration = self._durations.get(gate, physical_qubit) | ||
| dd_sequence_duration += gate.duration |
There was a problem hiding this comment.
Would it make sense to do this outside the for loop, since it should be static based on the configured dd sequence we don't need to recompute it for every delay.
There was a problem hiding this comment.
Lauren mentioned the same thing above. The duration is different for every qubit, but yes same across differernt delays of the same qubit. I did this in fa9da78
There was a problem hiding this comment.
LGTM, thanks for making the updates from the earlier reviews. Just one tiny comment inline but not a strong blocker. The only other thing I think is missing is a test case for _skip_reset_qubits with a Reset instruction (that would have caught the bug I found in my earlier review) but also not necessarily a blocker.
Co-authored-by: Lauren Capelluto <lcapelluto@users.noreply.github.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
ajavadia
force-pushed
the
dynamical-decoupling-pass
branch
from
b396404 to
d073585
Compare
Co-authored-by: Lauren Capelluto lcapelluto@users.noreply.github.com
Closes #6438
Summary
This commit adds a dynamical decoupling insertion pass. Given a scheduled circuit (i.e. a circuit where all the operations are annotated with a duration and where delays are inserted), it finds idle periods of time and inserts DD pulses. The circuit duration will remain the same. The user can select which qubits and what kind of spacing.
Details and comments