Skip to content
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

Fix layout output with hardware qubits #30

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

jakelishman
Copy link
Member

@jakelishman jakelishman commented Mar 12, 2024

This fixes the output of programs that involve hardware qubits to the form more standard to how Qiskit represents physical circuits. Now, the output circuit will have as many qubits as the maximum hardware qubit explicitly used, even if the lower qubits are not used. The TranspileLayout will be a mapping of the qubits to their own indices, to indicate that there were no underlying virtual qubits, but to assist with other tools in recognising that the circuit is defined on physical qubits.

The previous handling of hardware qubits created a circuit with only explicitly referenced qubits in the circuit, then attempted to convey the information about the actual hardware indices via the TranspileLayout. Unfortunately, this was not in a form that was readily understandable to other tools, and the model of the "physical" circuit not directly having its bit indices correspond to the hardware qubit index was at odds with Qiskit's usual model of physical circuits.

Fix #28


@jyu00: the resulting circuit won't be full device width (that would need an extra kwarg for the parser), but it'll match the expected behaviour that the used qubit indices in the dumped and loaded circuits will be the same. This appears to be fine through backend.run paths, and it's not rejected out-of-hand by EstimatorV2, but my test job of that on Peekskill (cqr71bd00evg008cxarg) just span its wheels stuck in "Running", so I cancelled it. This is the setup I used:

import re
from qiskit.circuit.random import random_circuit
from qiskit import QuantumCircuit, qasm2, qasm3, transpile
from qiskit_ibm_runtime import QiskitRuntimeService

service = QiskitRuntimeService()
backend = service.backend("ibm_peekskill")

random_circ = random_circuit(5, depth=3, seed=42).decompose(reps=3)
transpiled = transpile(random_circ, backend=backend, seed_transpiler=64, initial_layout=range(10, 15))

loaded = qasm3.loads(qasm3.dumps(transpiled))

# This works regardless of whether the full width is present:
assert loaded.num_qubits < backend.num_qubits
job = backend.run(loaded)

This fixes the output of programs that involve hardware qubits to the
form more standard to how Qiskit represents physical circuits.  Now, the
output circuit will have as many qubits as the maximum hardware qubit
explicitly used, even if the lower qubits are not used.  The
`TranspileLayout` will be a mapping of the qubits to their own indices,
to indicate that there were no underlying virtual qubits, but to assist
with other tools in recognising that the circuit is defined on physical
qubits.

The previous handling of hardware qubits created a circuit with only
explicitly referenced qubits in the circuit, then attempted to convey
the information about the actual hardware indices via the
`TranspileLayout`.  Unfortunately, this was not in a form that was
readily understandable to other tools, and the model of the "physical"
circuit not directly having its bit indices correspond to the hardware
qubit index was at odds with Qiskit's usual model of physical circuits.
@jyu00
Copy link

jyu00 commented Mar 13, 2024

Thanks @jakelishman for the quick update. However, I think this output would still be rejected by primitives (not sure why it wasn't in your test). QRT Primitives use the same ISA circuit check as qiskit-ibm-runtime, and it explicitly verifies the width of the input circuit is the same as the target.

@jakelishman
Copy link
Member Author

In this case, the circuit will have fewer qubits than the target, so it would pass that part of the test, I think? I had tried it with the Runtime's EstimatorV2, and I'm relatively sure I made it through that part of the test, I just got stymied by the particular backend I tried having an issue in its queue at the time. I can try again tomorrow.

Getting the output of the parser to match the exact width is something that will require a full feature release of Qiskit, because we'll need to extra an extra kwarg to the API to be able to specify the hardware number of qubits or something. That's totally plausible, but it would prevent this part of the patch going out in a patch release if I merge them together.

What I've suggested in other channels, if this isn't sufficient (which I think it might be anyway), is that as an immediate workaround for users direct-submitted OQ3, they can add a no-op rz(0.0) $126; somewhere in their circuit, and that'll force the output to be 127 qubits in all cases through this parser. That's obviously just a workaround while we add in the full feature support for telling the OQ3 parser how many qubits it should include in its output if it's a hardware circuit, but it should be totally safe.

@jakelishman
Copy link
Member Author

Jessie: I just tried again now the test backends are back online, and this code example worked end-to-end, despite the intermediate loaded circuit only using 15 hardware qubits out of 27 on Peekskill:

import re
from qiskit.circuit.random import random_circuit
from qiskit import QuantumCircuit, qasm2, qasm3, transpile
from qiskit_ibm_runtime import QiskitRuntimeService, EstimatorV2

service = QiskitRuntimeService()
backend = service.backend("ibm_peekskill")

random_circ = random_circuit(5, depth=3, seed=42).decompose(reps=3)
transpiled = transpile(random_circ, backend=backend, seed_transpiler=64, initial_layout=range(10, 15))

loaded = qasm3.loads(qasm3.dumps(transpiled))

# This works regardless of whether the full width is present:
assert loaded.num_qubits < backend.num_qubits

estimator = EstimatorV2(backend)
job = estimator.run([(loaded, "X"*loaded.num_qubits)])
job.result()[0].data.evs

So I don't even think my rz(0.0) $126; workaround suggestion will be needed.

@jyu00
Copy link

jyu00 commented Mar 14, 2024

Sorry I totally misread the code I linked myself! I read it as circuit.num_qubits != target.num_qubits instead of >. Thanks for running the test to ensure it works.

@jakelishman
Copy link
Member Author

No worries - it's definitely good to get this stuff tested before it's deployed. This PR is eligible for a patch release of qiskit-qasm3-import, which I'll try to get deployed immediately after this PR merges, and then the Runtime images can update to use it; there's no need to update Qiskit itself, since the current qiskit.qasm3.loads just wraps this library.

Copy link

@kevinhartman kevinhartman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jakelishman jakelishman merged commit a56e970 into Qiskit:main Mar 14, 2024
6 checks passed
@jakelishman jakelishman deleted the implied-qubit-width branch March 14, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Imported transpiled circuit has different layout
3 participants