adding an option to CouplingMap.reduce to allow disconnected coupling maps #10863
Conversation
|
One or more of the the following people are requested to review this:
|
There was a problem hiding this comment.
This LGTM, this is straightforward and a good addition to both not change the documented behavior and enable broader use of the reduce method. This was something we actually discussed in #9710 but rolled it back because it was too big an api change for an already large PR.
I had one inline comment about an improvement to efficiency we can make. But feel free to tag this as auto merge and we can make that change in follow up PR.
| @@ -281,17 +279,17 @@ def reduce(self, mapping): | |||
| if edge[0] in mapping and edge[1] in mapping: | |||
There was a problem hiding this comment.
This is pre-existing but we don't have to fix it here. But this would be more efficient if we did this as a set lookup because this will traverse the mapping list twice for each for each edge. rustworkx has a subgraph()` method on graphs, but it doesn't preserve the order of the nodes it receives. But we can at least get a speedup here if we did something like:
subgraph_nodes = set(mapping)
for edge in self.get_edges():
if edge[0] in subgraph_nodes and edge[1] in subgraph_nodes:|
Thanks! I have tried the performance improvement suggestion along with other strategies, such as creating a bitmask for For large graphs with many edges, it seems that > 95% of the time is spent on iterating over the edges ( |
Summary
This PR adds an option
check_if_connectedtoCouplingMap.reduce, defaulted toTrue. This corresponds to the previous behavior, checking whether the reduced coupling map remains connected and raising aCouplingErrorif not so. When set toFalse, the check is skipped, allowing disconnected reduced coupling maps. This is useful for synthesis plugins (in particular for #10657) where no connectivity assumptions are necessary (for example, we may want to collect and to resynthesize blocks of consecutive swap gates).Details and comments
This blocks #10657.
The check whether the reduced coupling map is (weakly) connected is now performed only when the option
check_if_connectedis set toTrue.The actual check now uses the
CouplingMap's methodis_connectedinstead of scipy. There was no noticeable change in performance.One additional minor point is that the reduced coupling map should have the correct number of vertices and either
coupling_map.add_physical_qubitorcoupling_map.graph.add_nodeshould be used. Previously nodes with no edges would not have been added.