[WIP] Hacking sabre_swap to work on the DAGDependency graph

[alexanderivrii]

Summary

Following up on the discussion from qiskit-dev-synthesis meeting, I have hacked the code in sabre_swap.py and sabre_dag.rs to work on both DAGCircuit and DAGDependency, thus having the option to exploit commutativity between gates, as was also suggested in #6247 and in https://arxiv.org/pdf/2202.03459.pdf.

The few required changes were as follows:

• The constructor of SabreDAG in sabre_dag.rs now explicitly receives the list of edges (instead of reconstructing the edges based on qubits and clbits involved, basically assuming that the DAG is a DAGCircuit).
• The construction of edges is now in sabre_swap.py, both for DAGCircuit and DAGDependency, and is based on examining nodes' direct predecessors and direct successors.
• SabreSwap.run() accepts an additional argument do_commutative_analysis (which is False by default). When True, it builds a helper DAGDependency out of the DAGCircuit passed to it.
• The main SABRE code does not depend on the original DAG structure with absolutely no changes being required.

There is also a temporary file sabre_experiments.py which can run SabreSwap with various options and checks the equivalence between the original and the transpiled circuits (following the ideas from #8597). This seems to work, except that:

• I am not sure if the Rust code is completely correct (to get the code working, I ended up removing NodeIndex::new(...) lines and randomly adding "&" symbols)
• I am not sure if examining direct predecessors and successors fully captures dependency of gates with classical bits on each other
• This resulted in some ugly code duplication in sabre_swap.py.

Feedback would be appreciated.

[qiskit-bot]

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:

• @Qiskit/terra-core
• @kevinhartman
• @mtreinish

[alexanderivrii]

Here are some quick benchmarking results on some of the red-queen's mapping/benchmarks/misc problems (but note: despite recent optimizations, on the very large circuits, DAGDependency is still extremely slow to construct, so I did not run on benchmarks with more than 10,000 gates). In the table below, NQ, 1Q and 2Q are the number of qubits, single-qubit gates, and two-qubit gates in the original circuit, the following 6 columns list the number of swaps introduced by Sabre for the linear nearest neighbor coupling map, when running with one of [B]asic, [L]ookahead and [D]ecay heuristics, either without or with [C] commutativity analysis (e.g., the column labeled "L -C" lists the number of SWAPs for lookahead heuristic without commutativity analysis). I have not experimented with the number of trials or other interesting SabreSwap options.

NAME                      NQ    1Q      2Q       |     B -C    B +C    L -C    L +C    D -C    D +C
0410184_169.qasm              14     107     104 |       68      53      60      50      60      50
3_17_13.qasm                   3      19      17 |        8       5       7       6       7       6
4gt10-v1_81.qasm               5      82      66 |       55      42      38      34      38      34
4gt11_82.qasm                  5       9      18 |       10       8      10       7      10       7
4gt11_83.qasm                  5       9      14 |        6       5       6       5       6       5
4gt11_84.qasm                  5       9       9 |        4       3       5       3       5       3
4gt12-v0_86.qasm               6     135     116 |       75      63      69      64      69      64
4gt12-v0_87.qasm               6     135     112 |       72      61      67      63      67      63
4gt12-v0_88.qasm               6     108      86 |       68      58      55      53      55      53
4gt12-v1_89.qasm               6     128     100 |       91      73      72      70      72      70
4gt13-v1_93.qasm               5      38      30 |       19      16      17      15      17      15
4gt13_90.qasm                  5      54      53 |       31      29      26      21      26      21
4gt13_91.qasm                  5      54      49 |       29      29      24      20      24      20
4gt13_92.qasm                  5      36      30 |       23      16      20      18      20      18
4gt4-v0_72.qasm                6     145     113 |       86      73      79      69      80      69
4gt4-v0_73.qasm                6     216     179 |      131     115     113      99     113      99
4gt4-v0_78.qasm                6     126     109 |       85      61      69      66      69      66
4gt4-v0_79.qasm                6     126     105 |       80      60      64      62      64      62
4gt4-v0_80.qasm                6     100      79 |       62      52      54      50      54      50
4gt4-v1_74.qasm                6     154     119 |       95      83      77      69      77      69
4gt5_75.qasm                   5      45      38 |       27      18      22      18      22      18
4gt5_76.qasm                   5      45      46 |       32      21      24      19      24      19
4gt5_77.qasm                   5      73      58 |       41      33      34      32      34      32
4mod5-bdd_287.qasm             7      39      31 |       28      22      24      24      24      24
4mod5-v0_18.qasm               5      38      31 |       21      18      21      17      21      17
4mod5-v0_19.qasm               5      19      16 |       12      10      12      11      12      12
4mod5-v0_20.qasm               5      10      10 |        5       4       5       4       5       4
4mod5-v1_22.qasm               5      10      11 |        5       5       6       4       6       4
4mod5-v1_23.qasm               5      37      32 |       26      19      22      14      22      14
4mod5-v1_24.qasm               5      20      16 |       13       9      11       9      11       9
4mod7-v0_94.qasm               5      90      72 |       53      43      41      39      41      39
4mod7-v1_96.qasm               5      92      72 |       51      37      44      38      44      38
4_49_16.qasm                   5     118      99 |       66      55      60      54      60      54
adr4_197.qasm                 13    1941    1498 |     1741    1464    1239    1167    1243    1161
aj-e11_165.qasm                5      82      69 |       45      36      42      33      42      33
alu-bdd_288.qasm               7      46      38 |       36      32      30      24      30      24
alu-v0_26.qasm                 5      46      38 |       28      21      23      21      23      21
alu-v0_27.qasm                 5      19      17 |       10       9      10       9      10       9
alu-v1_28.qasm                 5      19      18 |       10       9       9       9       9       9
alu-v1_29.qasm                 5      20      17 |       10       7       9       8       9       8
alu-v2_30.qasm                 6     281     223 |      187     146     150     134     150     134
alu-v2_31.qasm                 5     253     198 |      153     118     122     108     122     108
alu-v2_32.qasm                 5      91      72 |       54      46      46      43      46      43
alu-v2_33.qasm                 5      20      17 |       10       9       9       8       9       8
alu-v3_34.qasm                 5      28      24 |       17      16      15      11      15      11
alu-v3_35.qasm                 5      19      18 |       12       9      10       9      10       9
alu-v4_36.qasm                 5      64      51 |       30      24      26      24      26      24
alu-v4_37.qasm                 5      19      18 |       10       9      11      11      11      11
C17_204.qasm                   7     262     205 |      185     163     138     133     138     133
cm152a_212.qasm               12     689     532 |      517     455     375     373     375     373
cm42a_207.qasm                14    1005     771 |      823     658     555     537     553     536
cm82a_208.qasm                 8     367     283 |      252     234     187     180     187     180
cnt3-5_179.qasm               16      90      85 |       60      49      65      66      65      66
cnt3-5_180.qasm               16     270     215 |      172     145     134     117     134     117
con1_216.qasm                  9     539     415 |      421     355     298     300     294     289
cycle10_2_110.qasm            12    3402    2648 |     3194    2728    2015    1933    2023    1901
dc1_220.qasm                  11    1081     833 |      873     734     611     591     611     588
dc2_222.qasm                  15    5331    4131 |     5176    4421    3310    3189    3268    3197
decod24-bdd_294.qasm           6      41      32 |       18      15      16      14      16      14
decod24-enable_126.qasm        6     189     149 |      104      99      91      75      91      75
decod24-v0_38.qasm             4      28      23 |       14      12      14      12      14      12
decod24-v1_41.qasm             5      47      38 |       27      22      22      19      22      19
decod24-v2_43.qasm             4      30      22 |       14      11      13      11      13      11
decod24-v3_45.qasm             5      86      64 |       49      43      37      35      37      35
ex-1_166.qasm                  3      10       9 |        3       3       4       4       4       4
ex1_226.qasm                   6       2       5 |        6       4       8       4       8       4
ex2_227.qasm                   7     356     275 |      229     201     191     197     193     197
ex3_229.qasm                   6     228     175 |      157     118     117     113     117     113
f2_232.qasm                    8     681     525 |      521     434     384     367     384     367
graycode6_47.qasm              6       0       5 |        0       0       0       0       0       0
ham15_107.qasm                15    4905    3858 |     4352    3626    2948    2757    2936    2780
ham3_102.qasm                  3       9      11 |        4       3       5       5       5       5
ham7_104.qasm                  7     171     149 |      131     107      89      90      89      90
hwb4_49.qasm                   5     126     107 |       70      58      57      51      57      51
hwb5_53.qasm                   6     738     598 |      493     423     400     346     400     346
hwb6_56.qasm                   7    3771    2952 |     2733    2313    2036    1854    2050    1854
ising_model_10.qasm           10     390      90 |        0       0       0       0       0       0
ising_model_13.qasm           13     513     120 |        0       0       0       0       0       0
ising_model_16.qasm           16     636     150 |        0       0       0       0       0       0
majority_239.qasm              7     345     267 |      244     205     160     156     160     156
miller_11.qasm                 3      27      23 |       10       9      12      10      12      10
mini-alu_167.qasm              5     162     126 |       98      74      79      73      79      73
mini_alu_305.qasm             10      96      77 |       67      54      59      56      59      56
misex1_241.qasm               15    2713    2100 |     2441    2025    1605    1665    1623    1660
mod10_171.qasm                 5     136     108 |       81      62      65      65      65      65
mod10_176.qasm                 5     100      78 |       54      43      49      45      49      45
mod5adder_127.qasm             6     316     239 |      203     164     165     150     165     153
mod5d1_63.qasm                 5       9      13 |       10       7      10       7      10       7
mod5d2_64.qasm                 5      28      25 |       19      16      16      14      16      14
mod5mils_65.qasm               5      19      16 |       13      11      12      12      12      12
mod8-10_177.qasm               6     244     196 |      159     118     130     127     130     133
mod8-10_178.qasm               6     190     152 |      109     107     101      94     101      98
one-two-three-v0_97.qasm       5     162     128 |       99      80      78      65      78      65
one-two-three-v0_98.qasm       5      81      65 |       48      38      41      34      41      34
one-two-three-v1_99.qasm       5      73      59 |       39      37      36      30      36      30
one-two-three-v2_100.qasm      5      37      32 |       19      16      17      15      17      15
one-two-three-v3_101.qasm      5      38      32 |       22      18      17      17      17      17
pm1_249.qasm                  14    1005     771 |      823     658     555     537     553     536
qft_10.qasm                   10     110      90 |       49      36      44      46      44      46
qft_16.qasm                   16     272     240 |      140     105     116     170     116     171
radd_250.qasm                 13    1808    1405 |     1590    1353    1085    1035    1125    1039
rd32-v0_66.qasm                4      18      16 |       10       9       9       9       9       9
rd32-v1_68.qasm                4      20      16 |       10       9       9       9       9       9
rd32_270.qasm                  5      48      36 |       28      17      22      19      22      19
rd53_130.qasm                  7     595     448 |      404     339     300     286     300     288
rd53_131.qasm                  7     269     200 |      189     158     141     131     141     131
rd53_133.qasm                  7     324     256 |      221     177     155     151     156     152
rd53_135.qasm                  7     162     134 |      122     109      91      91      91      91
rd53_138.qasm                  8      72      60 |       52      43      43      37      43      37
rd53_251.qasm                  8     727     564 |      532     444     377     357     381     357
rd53_311.qasm                 13     151     124 |      150     118     116     111     114     111
rd73_140.qasm                 10     126     104 |      100      83      79      69      79      69
rd73_252.qasm                 10    3002    2319 |     2584    2222    1806    1689    1772    1668
rd84_142.qasm                 15     189     154 |      172     139     124     115     124     115
sf_274.qasm                    6     445     336 |      262     235     226     185     226     185
sf_276.qasm                    6     442     336 |      274     220     227     192     227     192
sqrt8_260.qasm                12    1695    1314 |     1589    1361    1087    1037    1089    1053
squar5_261.qasm               13    1124     869 |      995     830     699     653     705     745
square_root_7.qasm            15    4541    3089 |     3858    3265    3014    2513    3019    2567
sym6_145.qasm                  7    2187    1701 |     1562    1320    1038    1011    1035    1039
sym6_316.qasm                 14     147     123 |      161     145     129     120     130     119
sym9_146.qasm                 12     180     148 |      154     122     114     104     114     104
sys6-v0_111.qasm              10     117      98 |       88      83      73      69      73      69
wim_266.qasm                  11     559     427 |      441     371     321     310     325     312
xor5_254.qasm                  6       2       5 |        6       4       8       4       8       4
z4_268.qasm                   11    1730    1343 |     1488    1189    1063    1021    1061    1028

[coveralls]

Pull Request Test Coverage Report for Build 3282210238

• 29 of 46 (63.04%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.02%) to 84.739%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/transpiler/passes/routing/sabre_swap.py	23	40	57.5%

Totals
Change from base Build 3277989780:	-0.02%
Covered Lines:	61939
Relevant Lines:	73094

---

💛 - Coveralls

[jakelishman]

I think we should consider at a larger scale how we might want to add DAGDependency to paths within PassManager or above, without breaking the current BasePass interface which defines that the input to run is DAGCircuit, and the input to __call__ is QuantumCircuit. I'm also concerned with fitting this into other work, where we eventually want to be replacing the IR of the transpiler with some form of classical control-flow graph - I don't want us to start adding half-done features to certain transpiler passes without having a clear path in mind.

I don't think it's a good idea to overload run like this; as here, it ends up requiring every pass owner to manually implement their own dispatchers, or have messy nested code. From a typing perspective, we can't really apply standard OO principles to the methods any more, because we can't have BasePass take run(data: DAGCircuit | DAGDependency) without violating the typing of all existing passes (this is essentially the same problem as the first paragraph).

Ignoring the potential CFG change to the IR, off the top of my head, I would consider adding more than one "run" method that passes can define - one for DAGCircuit (probably needs to still be called run for backwards compatibility) and one for DAGDependency. PassManager can then call what's best at the given time, and do any conversions that might be necessary.

[jakelishman]

Just to expand a bit more on the code: the Rust-level changes of decoupling "edges" from qubits/clbits looks fine to me (and I'm excited to see the immediate improvements in size/depth!), and the way it's been done at the technical level generally looks ok. It has some implications for some work of mine (unpublished) that massively speeds up SabreSwap for large circuits, but I can easily reconcile that.

On the Python side: I think far more of the Python parts of the SabreSwap pass will need to be rewritten to make it efficient for consuming / producing a DAGDependency. At the moment, we essentially are throwing out all the commutation information we know from the input when we're reconstructing the output, which means apply_operation_back is going to have to it itself. We should be able to do better than that; we should be able to know efficiently where a given swap will go, and what the effects on subsequent commutation relations are. I do wonder if outputting DAGDependency would be sensible from a routing pass at all; on the surface at least, routing does appear to enforce a particular order that means the commutation grouping might not be a good choice. If that's true, it's another thing to consider in the interface of how the PassManager flow will look like; routing would appear to be special in that it wants to provide a DAGDependency -> DAGCircuit path, whereas everything else will most likely want T -> T.

[alexanderivrii]

@jakelishman, thanks for the very thorough feedback. Below are a few (subjective) thoughts on the subject.

I completely agree that from the perspective of a compiler, having multiple internal representations (IRs) of a circuit, allowing the transpiler flow to switch between different representations, and having different transpiler passes for different internal representations seems like a great approach.

However, despite me trying to use DAGDependency data structure in multiple places, I am not fully convinced that DAGDependency makes a great internal representation, and that we should push to extend some of the transpiler passes to DAGDependency.

On the positive side, DAGDependency does allow to exploit commutativity relations between individual gates in the circuit, allowing better choices in multiple algorithms (e.g., collecting linear gates exploiting gate commutativity, better gate-count/depth for sabre, etc.).

On the negative side, constructing a DAGDependency requires O(G^2) commutativity checks in the worst case, where G is the number of gates. Even worse than this, the DAGDependency graph may have O(G^2) edges. As a very simple example, consider a single-qubit circuit consisting of N X-gates, followed by N Y-gates:

X - X - ... - X - Y - Y - ... - Y

This very simple circuit has G = 2N-1 gates. However, as all X-gates commute with each other, and all Y-gates commute with each other, while none of the X-gates commute with any of Y-gates, the resulting DAGDependency graph will have N^2 edges. Personally, I don't think that we should consider IRs that require superlinear time to convert from/to DAGCircuit.

There are a few more caveats about DAGDependency. First, it only captures single-gate commutativity, and does not capture more complex commutativity relations, such as a gate commuting with a block of gates. For instance, in X - Y - Z none of the gates commute with each other, yet X commutes with the block Y - Z. Thinking ahead, some of the transpiler passes (such as template optimization) could try to capture these more powerful commutativity relations, so DAGDependency may not be an IR we want to have in the long run. It also seems that it's impossible to do local changes to DAGDependency, such as rewriting one sequence of gates into a different sequence of gates (of length more than 1). As a simple example, suppose that we have a single-qubit circuit consisting of N X-gates. The DAGDependency thus has N disjoint vertices and no edges. Now suppose we want to rewrite one of these X-gates as - Z * Y. This looks like a local change, however since none of the X-gates commute with the newly introduced Z and Y gates, we will need to add appropriate edges from every remaining X gate in the circuit.

I believe DAGDependency is still quite useful as a temporary auxiliary structure, but I wouldn't advocate to make an IR out of this. Maybe it could only be used with high optimization_level and only when the number of gates in the circuit does not exceed a certain predefined amount (say 1000 gates). Or maybe we could think of some kind of a window-based approach, where we would iteratively convert to DAGDependency blocks of at most 1000 gates, do some optimizations on each block, and convert the blocks back to DAGCircuit.

If we decide not to create an IR out of DAGDependency, then I still think that a PR like this one is useful. Sabre is the main routing algorithm in Qiskit today, and we should try to benefit from optimizations that reduce gate-count and depth. But again, I don't think we should turn on do_commutative_analysis in Sabre by default, maybe only with high optimization level and when there are not too many gates.

What do you think?

[alexanderivrii]

@jakelishman, do you agree that we do not want to make DAGDependency into an IR, and that it would be nice to have the option of letting Sabre work on the DAGs coming from DAGDependency?

[jakelishman]

I don't think we've got any solid decisions on how the DAGDependency structures will be present in / affect whatever we settle on for the transpiler IR. That said, were some DAGDependency-like object to be in the IR, I wouldn't have a problem with extending Sabre to handle it, and I don't think there are any algorithmic / technical reasons why we wouldn't be able to.

[alexanderivrii]

Thanks @jakelishman. I am happy to close this PR or to put it on hold until your PR #9012 is merged, whichever you think it's best.

[alexanderivrii]

I am closing this PR. The Sabre algorithm has significantly changed since this point, and the DAGDependency idea will likely require a somewhat different implementation.