nonuniform cost of physical swaps in sabre#9904
Conversation
This commit updates all the layout and routing passes which leverage hardware constraints to have a new optional keyword argument, target, to specify a Target object for modelling the constraints of the target backend. This option will superscede any other specified arguments for backend constraints. For most of these passes the target argument is just internally leveraged to build a CouplingMap (and in 2 cases a BackendProperties) as most of the algorithms don't need more than the global connectivity graph and there is minimal overhead to convert from a target to a CouplingMap. The 2 passes which take backend properties should be refactored to work natively with the target because the conversion adds extra overhead which isn't needed, but in those cases the passes aren't enabled by default (or are slated to be moved out of tree as a plugin, see Qiskit#8662) so the extra overhead isn't of particular concern. This is part of the first step towards making all backend constraints for the transpiler used the Target internally (see Qiskit#9256). Once all passes that take hardware constraints parameters as an input are updated to have a target we can start internally using the Target only for all preset pass manager construction and start the long process of deprecating the legacy interface in these passes. Related to Qiskit#9256
The pass manager drawer tests were comparing the dot output for statically built pass managers to test that the output visualization is generated correctly. However, the new arguments for the taget are changing the visualization (to show the new option) and caused these tests to fail. This commit updates the reference images to include the new argument.
This commit optimizes the logic of the CheckMap pass. Previously we were unecessarily creating a distance matrix from the coupling graph, and with a target unecessarily creating a CouplingMap object. Instead of doing this instead the pass now creates a list of bidirectional edges for adjacent nodes in the coupling graph and then a set lookup is done for each check instead of checking for a distance of 1 in the distance matrix.
This commit updates the pass constructors to use a single positional argument for both the coupling map or a target. This replaces the previous behavior where a new keyword argument was added to pass in the target. The exception to this is passes where the target was used differently than the coupling map for extra functionality.
|
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:
|
| print(dist_matrix) | ||
|
|
||
| if self.target: | ||
| dist_matrix = rx.digraph_floyd_warshall_numpy( |
There was a problem hiding this comment.
Do you think that it makes sense to wrap this in a try/except if the target basis isn't known to TwoQubitWeylDecomposition and fall back to the default distance matrix? I'm just wondering what happens if the decomposer doesn't handle the 2q gates in the basis?
There was a problem hiding this comment.
yeah you are right i still need to write test cases for the different cases we may encounter
| self.routing_pass.fake_run = False | ||
| return dag | ||
|
|
||
| dist_matrix = self.coupling_map.distance_matrix |
There was a problem hiding this comment.
We might want to put this in an else condition because this will probably compute the distance matrix (it is cached so if it was used before it won't) which can be expensive depending on the size of the graph. We probably don't want to do build two distance matrices if we're only going to use one.
|
The other question I just thought of is how does this interact with |
|
I'll close this as stale - we'd have to completely rewrite the PR if it were going to merge. |
Summary
Modifying the distance matrix between physical qubits in SabreLayout to account for the cost of swapping each pair of qubits. This cost is inferred from Target by looking at the available set of gates on that pair of qubits, their errors, and how many of them are needed to do a SWAP. The scoring of each swap is thus affected by the information in Target. Requires #9263.
Details and comments
Example:
error SWAP(0, 1) = error SWAP(1, 2)

error SWAP(0, 1) > error SWAP(1, 2)

error SWAP(0, 1) < error SWAP(1, 2)
