Context-aware dynamical decoupling #12825

Cryoris · 2024-07-26T14:59:53Z

Summary

Implement context-aware dynamical decoupling (DD) from arXiv:2403.06852.

Details and comments

When selecting decoupling sequences for an idle qubit, context-aware DD takes into account what actions are occuring on neighboring qubits. It ensures that neighboring sequences are mutually orthogonal (to avoid collisions, like in staggered DD) but also whether the qubit is a CX or ECR target/control spectator. This is nicely summarized in a figure of the paper, where the different wire colors denote different decoupling sequences:

This particularly leads to improvement if a lot of qubits are idle at the same time or if they are next to ECR/CX gates. One important category of circuits in this class are the layers in twirled circuits. This pass also works best if the ECR/CX gate times are the same for all connections, which is almost always the case on IBM devices.

Some simple experiments show that this pass performs better than the current standard in Qiskit, PadDynamicaDecoupling with an X-X sequence. Here's some basic benchmarks, see the paper for more:

This code is based off @kdk and @ajavadia 🙂

Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com> Co-authored-by: Ali Javadi <ajavadia@princeton.edu>

qiskit-bot · 2024-07-26T14:59:58Z

One or more of the following people are relevant to this code:

@Qiskit/terra-core

coveralls · 2024-07-30T14:10:04Z

Pull Request Test Coverage Report for Build 15410851778

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

356 of 381 (93.44%) changed or added relevant lines in 4 files are covered.
1208 unchanged lines in 26 files lost coverage.
Overall coverage increased (+0.04%) to 87.874%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/transpiler/passes/scheduling/padding/context_aware_dynamical_decoupling.py	353	378	93.39%

Files with Coverage Reduction	New Missed Lines	%
qiskit/circuit/library/pauli_evolution.py	2	96.49%
qiskit/result/sampled_expval.py	2	93.55%
crates/qasm2/src/lex.rs	3	92.48%
qiskit/transpiler/passes/routing/sabre_swap.py	3	96.97%
crates/cext/src/exit_codes.rs	4	0.0%
qiskit/synthesis/evolution/suzuki_trotter.py	4	90.38%
qiskit/synthesis/multi_controlled/mcx_synthesis.py	4	98.55%
crates/transpiler/src/passes/split_2q_unitaries.rs	5	96.62%
qiskit/synthesis/evolution/qdrift.py	5	86.49%
crates/accelerate/src/twirling.rs	6	97.33%

Totals
Change from base Build 15300566108:	0.04%
Covered Lines:	80816
Relevant Lines:	91968

💛 - Coveralls

qiskit/transpiler/passes/scheduling/padding/context_aware_dynamical_decoupling.py

done as the functions use data already available in the class-context, so we can avoid redundant passing of variables

miamico · 2024-10-18T19:50:43Z

Anything holding this up from being merged? Happy to lend a hand if needed to speed up the process @Cryoris @eliarbel

eliarbel

Thanks Julien, overall seems well written and documented. I didn't read the paper so the review is not about the actual algorithmic part of the DD method itself. I suggest getting input from Kevin or Ali if you want another pair of eyes on this aspect.
I've left line comments, most of which are minor but still require addressing.
In terms of performence - do you have a sense about the overhead of the block collection and manipulation (e.g. _collect_adjacenet_delay_blocks )?

qiskit/transpiler/passes/__init__.py

test/python/transpiler/test_context_aware_dd.py

qiskit/transpiler/passes/scheduling/padding/context_aware_dynamical_decoupling.py

ElePT

Thanks @Cryoris, as you know I have ran this pass and noticed a few painpoints I wanted to point out. In addition to the documentation suggestions, I have noticed that min_joinable_duration is a key parameter to avoid 1q gate explosion in some circuits, I wonder if you had considered setting it no a non-0 default value?

qiskit/transpiler/passes/scheduling/padding/context_aware_dynamical_decoupling.py

ElePT

Another comment, I believe there is a bug in the instruction duration assignation, because the timeline drawer fails after running this pass complaining about missing gate durations:

Traceback (most recent call last):
  File "/Users/ept/qiskit_workspace/qiskit/debug_dd.py", line 84, in <module>
    timeline_drawer(bound_scheduled_circuit, filename="out.png")
  File "/Users/ept/qiskit_workspace/qiskit/qiskit/utils/deprecation.py", line 183, in wrapper
    return func(*args, **kwargs)
  File "/Users/ept/qiskit_workspace/qiskit/qiskit/utils/deprecation.py", line 183, in wrapper
    return func(*args, **kwargs)
  File "/Users/ept/qiskit_workspace/qiskit/qiskit/visualization/timeline/interface.py", line 379, in draw
    canvas.load_program(program=program)
  File "/Users/ept/qiskit_workspace/qiskit/qiskit/visualization/timeline/core.py", line 193, in load_program
    for datum in gen(gate_source, formatter=self.formatter):
  File "/Users/ept/qiskit_workspace/qiskit/qiskit/visualization/timeline/generators.py", line 158, in gen_sched_gate
    if gate.duration > 0:
TypeError: '>' not supported between instances of 'NoneType' and 'int'

eliarbel

Thanks @Cryoris for applying all the changes. This looks good and I have no further comments. There are still some comments from @ElePT.

eliarbel

Fine from my side. Please address the remaining comments from @ElePT

Cryoris · 2024-12-03T17:07:56Z

#12825 (review) is a very good catch -- the newly added gates that implement the DD sequences didn't have a duration attached, since they weren't present when the scheduling pass was run. I'll add a test to check for this 🙂

mtreinish · 2024-12-04T00:44:40Z

#12825 (review) is a very good catch -- the newly added gates that implement the DD sequences didn't have a duration attached, since they weren't present when the scheduling pass was run. I'll add a test to check for this 🙂

Just so you know I'm working on removing per instruction durations in #13506 so we probably don't need to change this now. It would have been important for 1.3, but in case 2.0 we just making extra work for both of us.

Cryoris · 2024-12-04T13:11:13Z

I removed any setting and usage of Instruction.duration in 21c4ea0, so hopefully you don't have to update the context-aware DD in #13506 🙂 That means the timeline drawer will fail now, but I assume this will be fixed in your PR 🙂

eliarbel · 2025-05-05T11:11:03Z

After discussion with @ElePT I also updated the default behavior of min_joinable_duration: On realistic devices, the 2q gate lengths vary a little -- yet might not want to insert DD sequences for two concurrent 2q gates, just because they naturally differ slightly. The new default is therefore that we only add DD sequences for idle times beyond 2 * (max_2q_duration - min_2q_duration). This needs some benchmarking on real devices before merging.

Hi @Cryoris are you planning to benchmark the new min_joinable_duration default on real devices before the 2.1 release?

…yoris/qiskit-terra into context-aware-dynamical-decoupling

Cryoris · 2025-05-07T13:39:21Z

Yep, it's good from my side now 🙂 The number of X gates is rather large though, per design of the algorithm, I think that was just surprising to see.

E.g. for an N-local circuit with linear entanglement on n qubits (not that someone would run this), the number of X gates goes with O(n^2) instead of O(n) for "normal" DD. But the results of CA-DD look promising on the eagle devices and the overhead can be controlled with the min_joinable_duration parameter.

eliarbel · 2025-05-13T11:36:14Z

Overall this looks in good shape to me both from code and testing perspectives. I haven't reviewed the algorithm in depth against the paper, but I ran several examples and the results look OK. Still, I'd like to look into some of the algorithmic aspects a bit more. In the meantime, I have a couple questions and possible suggestions, both not major:

The rationale behind the default value for min_joinable_duration is not so clear to me. Also, it doesn't feel very intuitive as a knob for controlling the amount of DD inserted into the circuit. Is there some underlying intuition behind the choice, something that could be added to the docstring to help clarify this a bit?

'cz' is included in the calculation of the 2q gate length variance in _gate_length_variance. However, the logic for handling spectator qubits does not account for CZGates. Was this intentional, perhaps due to the differences in crosstalk levels between Heron and Eagle devices? If so, this introduces a technology assumption in the implementation, isn't it? I'm wondering if it would make sense to provide a way for users to opt in or out of spectator qubit analysis for specific gate types?

Cryoris · 2025-05-13T21:05:23Z

Re: min_joinable_duration -- this shouldn't set the amount of DD inserted, but rather control how adjacent idle blocks are joined. It's a knob that lets you trade-off between potentially many short sequences or longer blocks. The default value is picked to ensure that a layer of CX/ECR will not have a lot of short DD sequences just due to the difference in duration. But this is not discussed in the paper afaik and I'm happy to change this if you think there's a more reasonable choice 🙂

Re: CZ -- The paper only investigates CX/ECR. CZ would probably require a different spectator qubit handling, but this is not explained in the paper and hence not supported in the code. We can add an argument that allows to turn off the spectator-specific logic for CX/ECR.

eliarbel · 2025-05-15T07:28:52Z

Re: min_joinable_duration -- this shouldn't set the amount of DD inserted, but rather control how adjacent idle blocks are joined. It's a knob that lets you trade-off between potentially many short sequences or longer blocks. The default value is picked to ensure that a layer of CX/ECR will not have a lot of short DD sequences just due to the difference in duration. But this is not discussed in the paper afaik and I'm happy to change this if you think there's a more reasonable choice 🙂

The reason I treated this is as a knob to control the amount of DD is because of this check:

if (
    node.op.name == "delay"
    and not is_after_reset(node)
    and self._duration(node, qubit_map) > self._min_joinable_duration
   )

in _get_sorted_delays. It skips durations shorter than min_joinable_duration when collecting eligible delays, effectively becoming a filter of idle blocks based on their lengths. This is because shorter delays are not just omitted from adjacency blocks and have their own DD sequences, they are skipped altogether. If we want a truly min_joinable_duration behavior in the "joinable" sense, we'll probably want to handle the parameter in _collect_adjacent_delay_blocks. As for the value default value itself, I see the reasoning for layers so I think it's OK. But now I think that a name that better reflects how the logic is implemented currently is min_duration 😄

Cryoris · 2025-05-15T11:47:19Z

Fair point, let's rename it 👍🏻

eliarbel

Thanks @Cryoris for the patience with the final review. I only have 2 minor docs comments, otherwise this looks very good!

qiskit/transpiler/passes/scheduling/padding/context_aware_dynamical_decoupling.py

Thanks Eli! Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>

eliarbel

Great work - good to go! 🚀

* Context aware DD Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com> Co-authored-by: Ali Javadi <ajavadia@princeton.edu> * add reno * fix import * fix test by using GenericBackendV2 * use class method instead of standalones done as the functions use data already available in the class-context, so we can avoid redundant passing of variables * Eli's review comments * Elena's review comments * add details about time units * add gate.durations for inserted sequences * smarter default for min_joinable_durations this needs some benchmarking on real devices * lint + undo unwanted changes * don't use deprecated op.duration * fix tests after merge * handle `dt` none case * Fix tests after min duration default update * rename min_(joinable)_duration * Apply suggestions from code review Thanks Eli! Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com> --------- Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com> Co-authored-by: Ali Javadi <ajavadia@princeton.edu> Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>

Cryoris and others added 3 commits July 26, 2024 14:48

Context aware DD

e36bf43

Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com> Co-authored-by: Ali Javadi <ajavadia@princeton.edu>

add reno

e72837e

fix import

c056b10

Cryoris added Changelog: New Feature Include in the "Added" section of the changelog mod: transpiler Issues and PRs related to Transpiler labels Jul 26, 2024

Cryoris added this to the 1.3 beta milestone Jul 26, 2024

Cryoris requested a review from a team as a code owner July 26, 2024 14:59

fix test by using GenericBackendV2

e7b6097

eliarbel reviewed Jul 31, 2024

View reviewed changes

qiskit/transpiler/passes/scheduling/padding/context_aware_dynamical_decoupling.py Outdated Show resolved Hide resolved

Cryoris added 2 commits July 31, 2024 10:56

Merge branch 'main' into context-aware-dynamical-decoupling

b4c7be9

use class method instead of standalones

c6700eb

done as the functions use data already available in the class-context, so we can avoid redundant passing of variables

mtreinish modified the milestones: 1.3 beta, 1.3.0 Aug 1, 2024

eliarbel requested changes Oct 21, 2024

View reviewed changes

ElePT reviewed Oct 28, 2024

View reviewed changes

raynelfss assigned kdk, ajavadia and eliarbel Oct 29, 2024

ElePT reviewed Oct 30, 2024

View reviewed changes

Cryoris added 2 commits November 5, 2024 15:12

Eli's review comments

13802fc

Merge branch 'main' into context-aware-dynamical-decoupling

5618d62

raynelfss modified the milestones: 1.3.0, 2.0.0 Nov 7, 2024

eliarbel reviewed Nov 13, 2024

View reviewed changes

eliarbel previously approved these changes Nov 13, 2024

View reviewed changes

Merge branch 'main' into context-aware-dynamical-decoupling

9df11da

don't use deprecated op.duration

21c4ea0

1ucian0 unassigned kdk and ajavadia Jan 30, 2025

1ucian0 assigned Cryoris Feb 6, 2025

Cryoris modified the milestones: 2.0.0, 2.1.0 Mar 3, 2025

kevinhartman requested a review from eliarbel April 17, 2025 14:39

Merge branch 'main' into context-aware-dynamical-decoupling

e0dca73

Cryoris added 3 commits May 7, 2025 13:35

Merge branch 'main' into context-aware-dynamical-decoupling

a066720

fix tests after merge

37171ec

:wqMerge branch 'context-aware-dynamical-decoupling' of github.com:Cr…

d8432d1

…yoris/qiskit-terra into context-aware-dynamical-decoupling

Cryoris added 2 commits May 7, 2025 15:42

handle dt none case

eee5ad8

Fix tests after min duration default update

99522eb

Cryoris added 2 commits May 28, 2025 15:16

Merge branch 'main' into context-aware-dynamical-decoupling

8638631

rename min_(joinable)_duration

10fd4fb

eliarbel requested changes Jun 3, 2025

View reviewed changes

qiskit/transpiler/passes/scheduling/padding/context_aware_dynamical_decoupling.py Show resolved Hide resolved

qiskit/transpiler/passes/scheduling/padding/context_aware_dynamical_decoupling.py Outdated Show resolved Hide resolved

Apply suggestions from code review

bbebeae

Thanks Eli! Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>

eliarbel approved these changes Jun 3, 2025

View reviewed changes

Cryoris added this pull request to the merge queue Jun 3, 2025

Merged via the queue into Qiskit:main with commit e407edb Jun 3, 2025
26 checks passed

Context-aware dynamical decoupling #12825

Context-aware dynamical decoupling #12825

Uh oh!

Conversation

Cryoris commented Jul 26, 2024

Summary

Details and comments

Uh oh!

qiskit-bot commented Jul 26, 2024

Uh oh!

coveralls commented Jul 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 15410851778

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

Uh oh!

miamico commented Oct 18, 2024

Uh oh!

eliarbel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ElePT left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ElePT left a comment

Choose a reason for hiding this comment

Uh oh!

eliarbel left a comment

Choose a reason for hiding this comment

Uh oh!

eliarbel left a comment

Choose a reason for hiding this comment

Uh oh!

Cryoris commented Dec 3, 2024

Uh oh!

mtreinish commented Dec 4, 2024

Uh oh!

Cryoris commented Dec 4, 2024

Uh oh!

eliarbel commented May 5, 2025

Uh oh!

Cryoris commented May 7, 2025

Uh oh!

eliarbel commented May 13, 2025

Uh oh!

Cryoris commented May 13, 2025

Uh oh!

eliarbel commented May 15, 2025

Uh oh!

Cryoris commented May 15, 2025

Uh oh!

eliarbel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

eliarbel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coveralls commented Jul 30, 2024 •

edited

Loading