-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add qubit_subset option to BIPMapping pass #6756
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the approach looks good. And I agree with the mapping of qubits in the global_coupling_map and coupling_map to exist in BIPModel, so it can reason about physical error rates.
if self.global_qubit is None: | ||
self.global_qubit = list(range(self.coupling_map.size())) | ||
else: | ||
self.coupling_map = self.coupling_map.reduce(self.global_qubit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the difference between .reduce()
and .subgraph()
methods here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#6738 fixes a bug on .subgraph() btw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
subgraph()
does not retain the order of passed qubits while reduce()
does. For example (copied from #6736 (comment)),
coupling = CouplingMap.from_line(6, bidirectional=False)
list(coupling.subgraph([4, 2, 3, 5]).get_edges())
>>> [(0, 1), (1, 2), (2, 3)]
list(coupling.reduce([4, 2, 3, 5]).get_edges())
>>> [(1, 2), (2, 0), (0, 3)]
We need the map from reduced qubits (in coupling_map) to global qubits (in global_coupling_map) to recover the global qubits from reduced qubits. Using reduce()
, the map can be naturally represented as the list qubit_subset
(and that's why the type of qubit_subset
is list
(not set
) here).
Co-authored-by: Ali Javadi-Abhari <ajavadia@users.noreply.github.com>
…ko/qiskit-sdk-py into upgrade-bip-mapper-interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, and I'm happy that the pass doesn't call the 3 embedding stages now.
Summary
Add
qubit_subset
option toBIPMapping
pass to provide an easier way to specify physical qubits to be used in mapping.Details and comments
Current BIP mapper (added at #6580) requires users to supply a reduced coupling map which contains only the physical qubits to be used. That means it requires users to embed the circuit mapped on a reduced coupling map to the original coupling map as postprocessing. This seems a bit demanding. In addition, it also requires users to track virtual qubits -> qubits on reduced coupling map -> physical qubits and update
circuit._layout
by themselves to print the resulting circuit correctly.This PR enables to do the above things in one stop (without requiring postprocessing). It can be done by supplying
qubit_subset
with the original coupling map (reported by the backend). Users don't need to supply reduced coupling map any longer for specifying physical qubits to be used.For example, suppose the following mapping,
After this PR: The new
qubit_subset
option enables us to obtain the printable mapped circuit by one line:Before this PR: The current interface requires dozens of lines to obtain the same mapped circuit:
Alternatives considered
PartialCouplingMap
instead ofCouplingMap
CouplingMap
class does not accept "holes" in physical qubits. So we may need to introduce another coupling class to allow holes in physical qubits, sayPartialCouplingMap
. However,PartialCouplingMap
would be more likely to be a utility class for pass developers rather than interface class for pass users.