Starting layout analysis pass for sabre by alexanderivrii · Pull Request #10829 · Qiskit/qiskit

alexanderivrii · 2023-09-13T11:03:45Z

Summary

This PR augments #10721 with a specific strategy to create a starting sabre layout, implemented as SabrePreLayout analysis pass.

The pass works by augmenting the coupling map with more and more "extra" edges until VF2 succeeds to find a perfect graph isomorphism. More precisely, the augmented coupling map contains edges between nodes that are within a given distance d in the original coupling map, and the value of d is increased until an isomorphism is found.

Intuitively, a better layout involves fewer extra edges. The pass also optionally minimizes the number of extra edges involved in the layout until a local minimum is found. This involves removing extra edges and running VF2 to see if an isomorphism still exists.

Details and comments

Matthew (@mtreinish) had the idea to mitigate the problem of the transpiler yielding less optimal circuits on larger topologies (described in #10160) by creating "better" starting layouts for Sabre and using these in addition to the fully random layouts (see #10169 and #10721). This particular strategy of creating a "better" layout is heavily inspired by #10169.

Here are a few experiments on the EfficientSU2 example with circular entanglement from #10160. The quantum circuit is EfficientSU2(num_qubits=16, entanglement='circular', reps=6, flatten=True) and the coupling map is CouplingMap.from_heavy_hex(15).

Running Sabre yields the layout on the left. Note that certain pairs of qubits that are connected in the abstract circuit get quite separated in the physical circuit (shown using black arrows). Using SabrePreLayout before Sabre yields the layout in the middle. In this case there is a way to embed the abstract circuit such that all connected nodes will be at most distance-2 apart in the physical map, and Sabre ends up choosing this starting layout over the random layouts. Note however that this layout is still visually non-optimal, containing both a qubit away from the "figure-8" (circled pair of red qubits) and gaps in the "figure-8" (circles blue qubits). Using the additional optional minimization in SabrePreLayout yields the layout on the right, which is the best out of the 3.

Note that heavy-hex architectures a bit funny, in that they also contain heavy pentagons in addition to heavy hexagons (the pentagons are at the border of the coupling map). An even better layout would be a "figure-8" around a heavy-hexagon and a heavy-pentagon, but the proposed method is not guaranteed to find it.

Limitations of the approach

The proposed method is expected to work only for a niche set of applications, where a perfect layout does not exist, but a "close-to-perfect" layout does. For example, it would not be useful for abstract circuits where every pair of qubits is connected. However, the way we have organized the transpiler flow is that SabrePreLayout is just an analysis pass that creates additional layouts for Sabre, if these turn out to be not useful, Sabre will end up simply ignoring these.

qiskit-bot · 2023-09-13T11:03:48Z

One or more of the the following people are requested to review this:

@Qiskit/terra-core

coveralls · 2023-09-13T11:38:17Z

Pull Request Test Coverage Report for Build 6534170170

87 of 88 (98.86%) changed or added relevant lines in 3 files are covered.
7 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.01%) to 87.06%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/transpiler/passes/layout/sabre_pre_layout.py	85	86	98.84%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/expr.rs	1	93.76%
crates/qasm2/src/lex.rs	6	91.67%

Totals
Change from base Build 6529876136:	0.01%
Covered Lines:	74517
Relevant Lines:	85593

💛 - Coveralls

mtreinish

Thanks for making the updates, I just had a couple of other comments inline after taking another pass at review.

mtreinish · 2023-09-27T17:29:43Z

+        target=None,
+        coupling_map=None,


Could we combine these args (and just use target). We did something similar in SabreLayout:

qiskit/qiskit/transpiler/passes/layout/sabre_layout.py

Lines 164 to 169 in 18a9a2b

if isinstance(coupling_map, Target):

self.target = coupling_map

self.coupling_map = self.target.build_coupling_map()

else:

self.target = None

self.coupling_map = coupling_map

(but used coupling_map as the name for backwards compatibility).

I guess we could do this, but this feels like this is somewhat abusing notation. I don't feel very strongly one way or another.

I was just thinking it'd be good for consistency, but I also don't feel super strongly (I preferred the two args myself, but moved to the shared for approach based on review feedback in #9263 ). Personally I just want to implement #9256 so we don't have to worry about this dual path issue anymore.

Sure, it's better to be consistent across different passes, done in 96b1056.

mtreinish · 2023-09-27T17:36:47Z

+            d = self.coupling_map.distance(x, y)
+            if 1 < d <= distance:
+                augmented_coupling_map.add_edge(x, y)
+                augmented_error_map.add_error((x, y), self.error_rate)


I know we were talking offline about scaling the error rate based on the edge distance. Do you think doing something like:

Suggested change

augmented_error_map.add_error((x, y), self.error_rate)

augmented_error_map.add_error((x, y), self.error_rate * distance)

Or were you thinking of something more complicated?

I think something like this is a good idea, except that if error_rate * distance becomes larger than 1, then the final fidelity will be 0, regardless of the layout (because the scoring code multiplies all the fidelities). So either we can live with this (in the worst case we will get a crappy starting layout that Sabre will eventually not pick), or to automatically decrease error_rate to something like 1 / (2 * distance), or to consider some arctan-like function f(distance) such that f(1) = 0 (real edges have no error) and f(infinity)=1 (100% error), and everything else would be in-between.

I like the asymptotic error rate approach that seems like the right fit for this use case. Although, I'd probably just be lazy and do something like min(0.9999, self.error_rate * distance) to approximate it :)

This actually works well in practice, though at the end I have gone with an also simple asymptotic approach which resembles score calculations in sabre error_rate(d) = 1 - ( (1 - self.error_rate)^d ) (but interestingly gives the same results as this "lazy" heuristic).

I didn't know this existed :) Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

mtreinish · 2023-10-11T20:16:57Z

I think this is basically ready to go when resolve the question on the heuristic error for the added edges.

…qiskit-terra into initial-layout-for-sabre

alexanderivrii · 2023-10-12T10:06:19Z

I experimented quite a bit with mapping the BV circuit over 16 qubits to the heavy-hex(n) coupling for increasing values of n, and saw that (i) this PreSabreLayout helps even if the smallest d for which VF2 finds the layout is 4 (i.e. we need to add distance-4 edges to the coupling graph to find an embedding), (ii) additionally increasing error rates for longer-distance edges helps for larger values of n. In the end I have went with the heuristic that closely resembles computation of errors in sabre scoring: error_rate(d) = 1 - ( (1 - self.error_rate)^d ), though both this and Matthew's "lazy" heuristic error_rate(d) = min(0.9999, self.error_rate * distance) gave identical results. However now that I have merged main into the branch I see that the transpiler results became very different and much-much better due to other changes in Sabre (though I still don't quite understand the reason).

…y with other passes

mtreinish · 2023-10-12T21:30:46Z

+
+        if self.coupling_map is None:
+            raise TranspilerError(
+                "SabrePreLayout requires either target or coupling_map to be provided."


Suggested change

"SabrePreLayout requires either target or coupling_map to be provided."

"SabrePreLayout requires coupling_map to be used with either a "

"CouplingMap or Target."

Fixed in 5e6d3a5.

mtreinish · 2023-10-12T21:38:56Z

+            target (Target): A target representing the backend device. If specified, it will
+                supersede a set value for ``coupling_map``.


Suggested change

target (Target): A target representing the backend device. If specified, it will

supersede a set value for ``coupling_map``.

Oops. Fixed in 5e6d3a5.

mtreinish · 2023-10-12T21:39:28Z

+            coupling_map (Union[CouplingMap, Target]): directed graph representing the
+                original coupling map.


Suggested change

coupling_map (Union[CouplingMap, Target]): directed graph representing the

original coupling map.

coupling_map (Union[CouplingMap, Target]): directed graph representing the

original coupling map or a target modelling the backend (including its

connectivity).

Done in 5e6d3a5.

mtreinish · 2023-10-12T21:58:15Z

+    def _find_layout(self, dag, edges):
+        """Checks if there is a layout for a given set of edges."""
+        cm = CouplingMap(edges)
+        pass_ = VF2Layout(cm, seed=0, max_trials=1, call_limit=self.call_limit_vf2)


I feel like this is a bit heavy weight we're basically we just need to call rustworkx.is_subgraph_isomorphic(cm_graph, im_graph, id_order=True, induced=False, call_limit=self.call_limit_vf2) here without all the pass machinery to determine if a reduced edge list is valid or not.

For a first implementation I think this is fine, because there is probably some larger refinement we'll want to do since we do need a layout with the minimized edge list.

I am leaving this as is for now, but sure this is something to rethink in the future.

mtreinish · 2023-10-12T22:17:51Z

+                extra_edges_unprocessed_set = self._get_extra_edges_used(dag, layout).difference(
+                    set(extra_edges_necessary)
+                )
+                best_layout = layout


If we minimize the edge list we don't take into account noise really and just return the first edge found? Since _find_layout sets max_trials=1. Is there value in running multiple trials on the output of minimization to take into account error rates?

This is a good question. On the one hand, I believe that some part of the effort of looking for extra solutions at this stage would be wasted, as the top-level algorithm would proceed to removing more edges and calling this function again. On the other hand, it does seem a good idea to take the noise into account here as well. However, I do think that the effort here should be smaller than the effort in the main call. A possible solution would be to add yet another argument to the run function, something like improve_layout_max_trials_vf2 (and, while we are at this, also improve_layout_call_limit_vf2). Please tell me if you are agree with this.

Yeah, I'm not sure either way. In general for sabre I don't think noise awareness buys us very much because we have VF2PostLayout which factors in the whole circuit after routing. The place where this is different is to try and avoid the extra edges we've added to the coupling map. But I think for right now this is probably fine, we can always refine the pass more and add extra options in a follow up.

mtreinish

This LGTM, just one tiny inline nit (sorry I didn't catch it before) and then I think this is good to merge

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

jakelishman · 2023-10-16T16:14:59Z

Test failure in the merge queue was a network timeout with installing the dependencies - not a true failure, and hopefully just ephemeral.

Building on the work done in Qiskit#10829, Qiskit#10721, and Qiskit#12104 this commit adds a new trial to all runs of `SabreLayout` that runs the dense layout pass. In general the sabre layout algorithm starts from a random layout and then runs a routing algorithm to permute that layout virtually where swaps would be inserted to select a layout that would result in fewer swaps. As was discussed in Qiskit#10721 and Qiskit#10829 that random starting point is often not ideal especially for larger targets where the distance between qubits can be quite far. Especially when the circuit qubit count is low relative to the target's qubit count this can result it poor layouts as the distance between the qubits is too large. In qiskit we have an existing pass, `DenseLayout`, which tries to find the most densely connected n qubit subgraph of a connectivity graph. This algorithm necessarily will select a starting layout where the qubits are near each other and for those large backends where the random starting layout doesn't work well this can improve the output quality. As the overhead of `DenseLayout` is quite low and the core algorithm is written in rust already this commit adds a default trial that uses DenseLayout as a starting point on top of the random trials (and any input starting points). For example if the user specifies to run SabreLayout with 20 layout trials this will run 20 random trials and one trial with `DenseLayout` as the starting point. This is all done directly in the sabre layout rust code for efficiency. The other difference between the standalone `DenseLayout` pass is that in the standalone pass a sparse matrix is built and a reverse Cuthill-McKee permutation is run on the densest subgraph qubits to pick the final layout. This permutation is skipped because in the case of Sabre's use of dense layout we're relying on the sabre algorithm to perform the permutation. Depends on: Qiskit#12104

Building on the work done in #10829, #10721, and #12104 this commit adds a new trial to all runs of `SabreLayout` that runs the dense layout pass. In general the sabre layout algorithm starts from a random layout and then runs a routing algorithm to permute that layout virtually where swaps would be inserted to select a layout that would result in fewer swaps. As was discussed in #10721 and #10829 that random starting point is often not ideal especially for larger targets where the distance between qubits can be quite far. Especially when the circuit qubit count is low relative to the target's qubit count this can result it poor layouts as the distance between the qubits is too large. In qiskit we have an existing pass, `DenseLayout`, which tries to find the most densely connected n qubit subgraph of a connectivity graph. This algorithm necessarily will select a starting layout where the qubits are near each other and for those large backends where the random starting layout doesn't work well this can improve the output quality. As the overhead of `DenseLayout` is quite low and the core algorithm is written in rust already this commit adds a default trial that uses DenseLayout as a starting point on top of the random trials (and any input starting points). For example if the user specifies to run SabreLayout with 20 layout trials this will run 20 random trials and one trial with `DenseLayout` as the starting point. This is all done directly in the sabre layout rust code for efficiency. The other difference between the standalone `DenseLayout` pass is that in the standalone pass a sparse matrix is built and a reverse Cuthill-McKee permutation is run on the densest subgraph qubits to pick the final layout. This permutation is skipped because in the case of Sabre's use of dense layout we're relying on the sabre algorithm to perform the permutation. Depends on: #12104

Building on the work done in Qiskit#10829, Qiskit#10721, and Qiskit#12104 this commit adds a new trial to all runs of `SabreLayout` that runs the dense layout pass. In general the sabre layout algorithm starts from a random layout and then runs a routing algorithm to permute that layout virtually where swaps would be inserted to select a layout that would result in fewer swaps. As was discussed in Qiskit#10721 and Qiskit#10829 that random starting point is often not ideal especially for larger targets where the distance between qubits can be quite far. Especially when the circuit qubit count is low relative to the target's qubit count this can result it poor layouts as the distance between the qubits is too large. In qiskit we have an existing pass, `DenseLayout`, which tries to find the most densely connected n qubit subgraph of a connectivity graph. This algorithm necessarily will select a starting layout where the qubits are near each other and for those large backends where the random starting layout doesn't work well this can improve the output quality. As the overhead of `DenseLayout` is quite low and the core algorithm is written in rust already this commit adds a default trial that uses DenseLayout as a starting point on top of the random trials (and any input starting points). For example if the user specifies to run SabreLayout with 20 layout trials this will run 20 random trials and one trial with `DenseLayout` as the starting point. This is all done directly in the sabre layout rust code for efficiency. The other difference between the standalone `DenseLayout` pass is that in the standalone pass a sparse matrix is built and a reverse Cuthill-McKee permutation is run on the densest subgraph qubits to pick the final layout. This permutation is skipped because in the case of Sabre's use of dense layout we're relying on the sabre algorithm to perform the permutation. Depends on: Qiskit#12104

alexanderivrii added 4 commits September 13, 2023 12:36

adding SabreStartingLayoutUsingVF2 analysis pass

9a5f01b

fixing imports

625d857

bug fix

c244393

fixing test after changing some of the options

be118a6

alexanderivrii requested a review from a team as a code owner September 13, 2023 11:03

alexanderivrii added 3 commits September 15, 2023 08:47

renaming

6e9e113

adding target; more renaming; tests

7df9faf

release notes

3982e03

alexanderivrii changed the title ~~[WIP] starting layout analysis pass for sabre~~ Starting layout analysis pass for sabre Sep 15, 2023

mtreinish mentioned this pull request Sep 19, 2023

[WIP] Use VF2 to find a partial layout for seeding a SabreLayout trial #10169

Closed

4 tasks

mtreinish self-assigned this Sep 19, 2023

mtreinish added the Changelog: Added Add an "Added" entry in the GitHub Release changelog. label Sep 19, 2023

mtreinish added this to the 0.45.0 milestone Sep 19, 2023

mtreinish reviewed Sep 19, 2023

View reviewed changes

Comment thread test/python/transpiler/test_sabre_layout.py Outdated

alexanderivrii added 3 commits September 20, 2023 14:27

Merge branch 'main' into initial-layout-for-sabre

9ad9806

applying suggestions from code review

f006dcd

removing debug print

e02f859

alexanderivrii requested a review from mtreinish September 21, 2023 07:30

mtreinish reviewed Sep 27, 2023

View reviewed changes

alexanderivrii and others added 4 commits September 29, 2023 11:32

Update qiskit/transpiler/passes/layout/sabre_pre_layout.py

64e89d1

I didn't know this existed :) Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

adding missing :

c91fac1

collecting edges into a set

7235ef7

Merge branch 'main' into initial-layout-for-sabre

986b8e6

alexanderivrii added 2 commits October 12, 2023 12:21

adjusting error_rate with respect to distance

668e2bd

Merge branch 'initial-layout-for-sabre' of github.com:alexanderivrii/…

e7e010e

…qiskit-terra into initial-layout-for-sabre

letting coupling_map be either coupling map or target, for consistenc…

96b1056

…y with other passes

alexanderivrii requested a review from mtreinish October 12, 2023 13:57

mtreinish reviewed Oct 12, 2023

View reviewed changes

alexanderivrii added 2 commits October 16, 2023 12:52

Merge branch 'main' into initial-layout-for-sabre

2728310

apply suggestions from code review

5e6d3a5

mtreinish previously approved these changes Oct 16, 2023

View reviewed changes

Comment thread qiskit/transpiler/passes/layout/sabre_pre_layout.py Outdated

Update qiskit/transpiler/passes/layout/sabre_pre_layout.py

70754a5

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

alexanderivrii dismissed mtreinish’s stale review via 70754a5 October 16, 2023 13:16

mtreinish approved these changes Oct 16, 2023

View reviewed changes

mtreinish enabled auto-merge October 16, 2023 13:51

mtreinish added this pull request to the merge queue Oct 16, 2023

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 16, 2023

jakelishman added this pull request to the merge queue Oct 16, 2023

Merged via the queue into Qiskit:main with commit df9eae4 Oct 16, 2023

alexanderivrii deleted the initial-layout-for-sabre branch October 23, 2023 07:23

mtreinish mentioned this pull request May 22, 2024

Add DenseLayout trial to SabreLayout #12453

Merged

	if isinstance(coupling_map, Target):
	self.target = coupling_map
	self.coupling_map = self.target.build_coupling_map()
	else:
	self.target = None
	self.coupling_map = coupling_map

	augmented_error_map.add_error((x, y), self.error_rate)
	augmented_error_map.add_error((x, y), self.error_rate * distance)

	"SabrePreLayout requires either target or coupling_map to be provided."
	"SabrePreLayout requires coupling_map to be used with either a "
	"CouplingMap or Target."

		target (Target): A target representing the backend device. If specified, it will
		supersede a set value for ``coupling_map``.

		coupling_map (Union[CouplingMap, Target]): directed graph representing the
		original coupling map.

Conversation

alexanderivrii commented Sep 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details and comments

Limitations of the approach

Uh oh!

qiskit-bot commented Sep 13, 2023

Uh oh!

coveralls commented Sep 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 6534170170

💛 - Coveralls

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexanderivrii Sep 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mtreinish commented Oct 11, 2023

Uh oh!

alexanderivrii commented Oct 12, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jakelishman commented Oct 16, 2023

Uh oh!

Reviewers

Assignees

Labels

alexanderivrii commented Sep 13, 2023 •

edited

Loading

coveralls commented Sep 13, 2023 •

edited

Loading

alexanderivrii Sep 29, 2023 •

edited

Loading