-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Support for backends with defective qubits and gates #4782
Support for backends with defective qubits and gates #4782
Conversation
…e_qubits_and_gates
…e_qubits_and_gates
…e_qubits_and_gates
…e_qubits_and_gates
Test added. |
A side note, dropping the assumption that coupling maps always start with 0 (see discussion in #2314) would make this implementation much cleaner, I think. |
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.
So I have an inline question/concern but that's probably a larger refactor. If we really need to have this in 0.15.0 then I think this is fine. It's a bit hacky feeling, but since it's if blocked on faulty qubits being set we're not going to trigger the code normally so if there are issues we can resolve it later. That being said this should probably have a release note, unless you don't want to advertise it in the documentation yet.
def _remap_circuit_faulty_backend(circuit, num_qubits, backend_prop, faulty_qubits_map): | ||
faulty_qubits = backend_prop.faulty_qubits() if backend_prop else [] | ||
disconnected_qubits = {k for k, v in faulty_qubits_map.items() | ||
if v is None}.difference(faulty_qubits) | ||
faulty_qubits_map_reverse = {v: k for k, v in faulty_qubits_map.items()} | ||
if faulty_qubits: | ||
faulty_qreg = circuit._create_qreg(len(faulty_qubits), 'faulty') | ||
else: | ||
faulty_qreg = [] | ||
if disconnected_qubits: | ||
disconnected_qreg = circuit._create_qreg(len(disconnected_qubits), 'disconnected') | ||
else: | ||
disconnected_qreg = [] | ||
|
||
new_layout = Layout() | ||
faulty_qubit = 0 | ||
disconnected_qubit = 0 | ||
|
||
for real_qubit in range(num_qubits): | ||
if faulty_qubits_map[real_qubit] is not None: | ||
new_layout[real_qubit] = circuit._layout[faulty_qubits_map[real_qubit]] | ||
else: | ||
if real_qubit in faulty_qubits: | ||
new_layout[real_qubit] = faulty_qreg[faulty_qubit] | ||
faulty_qubit += 1 | ||
else: | ||
new_layout[real_qubit] = disconnected_qreg[disconnected_qubit] | ||
disconnected_qubit += 1 | ||
physical_layout_dict = {} | ||
for qubit in circuit.qubits: | ||
physical_layout_dict[qubit] = faulty_qubits_map_reverse[qubit.index] | ||
for qubit in faulty_qreg[:] + disconnected_qreg[:]: | ||
physical_layout_dict[qubit] = new_layout[qubit] | ||
dag_circuit = circuit_to_dag(circuit) | ||
apply_layout_pass = ApplyLayout() | ||
apply_layout_pass.property_set['layout'] = Layout(physical_layout_dict) | ||
circuit = dag_to_circuit(apply_layout_pass.run(dag_circuit)) | ||
circuit._layout = new_layout | ||
return circuit | ||
|
||
|
||
def _remap_layout_faulty_backend(layout, faulty_qubits_map): | ||
if layout is None: | ||
return layout | ||
new_layout = Layout() | ||
for virtual, physical in layout.get_virtual_bits().items(): | ||
if faulty_qubits_map[physical] is None: | ||
raise TranspilerError("The initial_layout parameter refers to faulty" | ||
" or disconnected qubits") | ||
new_layout[virtual] = faulty_qubits_map[physical] | ||
return new_layout |
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.
So the thing I don't really like about this is that this is baked into transpile()
instead of being a pass? It feels to me like this should be a pass that we add to the preset passmanagers during the layout phase. I assume this is just because CouplingMap
is an __init__
parameter to passes that use it, but I think this raises a larger question if coupling map isn't a static property for the life of a pass manager anymore then maybe we should be treating it differently in the data flow (like make it part of the property set).
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.
you assumption is correct. It use to be part of the design that passes cannot change "static" element like the coupling map. Maybe another thing to revisit.
Release note added in 777a350 |
…4782)" This PR was merged in as the last commit in the release as an admittedly less than ideal implementation but a first pass of a feature we deemed as necessary to include in the release. However, it has not been throghoughly tested enough and is blocking the release of the metapackage [1] because it breaks transpiling in the notebooks. This PR has been problematic and was already reverted once for a similar reason. This reverts commit a67440a to unblock the metapackage release we should sit down and thing about how to fit this feature into the compiler better before we attemp a third time. [1] https://travis-ci.com/github/Qiskit/qiskit/jobs/369663144#L732
fixes #4731
This is the second round for support for backends with defective qubits and gates. The first try was #4110, and @mtreinish helped me to understand that I cannot parametrize
parallel_map
with backend since that forces a backend to be pickable.This PR is basically as previous but, instead of using the full backend as a parameter, only takes the data from the backend that is needed. I also improve code so it supports multiple backends (when multiple circuits are transpiled) and considers the qubit/gate operability independently.
Thanks @mtreinish for investigating the issue and pointing me to the problem/solution!