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

Add ESP readout support to Qiskit #6543

Merged
merged 9 commits into from
Jun 14, 2021

Conversation

zachschoenfeld33
Copy link
Contributor

Summary

Adds flag use_measure_esp to assemble, which sets the final measurement in a circuit to use Excited State Promoted (ESP) readout. See https://arxiv.org/pdf/2008.08571.pdf for reference.

A backend configuration flag measure_esp_enabled is included. The ESP feature can only be used if measure_esp_enabled=True.

@zachschoenfeld33
Copy link
Contributor Author

@taalexander @dtmcclure

qiskit/compiler/assembler.py Outdated Show resolved Hide resolved
qiskit/compiler/assembler.py Outdated Show resolved Hide resolved
qiskit/compiler/assembler.py Outdated Show resolved Hide resolved
qiskit/execute_function.py Outdated Show resolved Hide resolved
qiskit/providers/models/backendconfiguration.py Outdated Show resolved Hide resolved
@taalexander taalexander merged commit 6d605a0 into Qiskit:main Jun 14, 2021
@mtreinish
Copy link
Member

I'm not clear why this needed to be merged into terra at all. It's an ibm backend only option right? Why isn't this just exposed as an option from the ibmq provider backend? It should be passed through the kwargs for the run command (and execute() by extension) if it's exposed through the options interface correctly.

Also we shouldn't be updating the schemas here anymore. They're deprecated and frozen slated for removal soon (see #6558 )

@taalexander
Copy link
Contributor

I'm not clear why this needed to be merged into terra at all. It's an ibm backend only option right? Why isn't this just exposed as an option from the ibmq provider backend? It should be passed through the kwargs for the run command (and execute() by extension) if it's exposed through the options interface correctly.

Agreed, this is a backend-only option. Unfortunately, the provider is calling out to assemble to build the Qobj payload still, so until this function is migrated to the provider this seems like the required place for this logic. We avoided any other IBM specific changes in this PR. See Qiskit/qiskit-ibmq-provider#952 for backend-specific changes.

Also we shouldn't be updating the schemas here anymore. They're deprecated and frozen slated for removal soon (see #6558 )

Ok, these changes are just a mirror of Qiskit/ibm-quantum-schemas#10. We will not make further updates to these schemas.

@mtreinish
Copy link
Member

I'm not clear why this needed to be merged into terra at all. It's an ibm backend only option right? Why isn't this just exposed as an option from the ibmq provider backend? It should be passed through the kwargs for the run command (and execute() by extension) if it's exposed through the options interface correctly.

Agreed, this is a backend-only option. Unfortunately, the provider is calling out to assemble to build the Qobj payload still, so until this function is migrated to the provider this seems like the required place for this logic. We avoided any other IBM specific changes in this PR. See Qiskit/qiskit-ibmq-provider#952 for backend-specific changes.

Sure, I agree that we should copy the assembler and qiskit.qobj over to the ibmq provider and longer term deprecate and remove it from terra (but we should be conservative on that given it's former central usage). But, even without that being done the way assemble() is written you don't need to add explicit kwargs for every single option in the run config. If you pass in arbitrary kwargs to the function it gets added directly the run config. For example I ran:

from qiskit.compiler import assemble
from qiskit import QuantumCircuit
import qiskit

print(qiskit.__version__)
qc = QuantumCircuit(2)
qc.h(0)
qc.cx(0, 1)
qc.measure_all()
print(assemble(qc, use_measure_esp=True))

with the current release of qiskit-terra and it returns:

0.17.4
QASM Qobj: a5005853-6158-4e00-9abe-9f061adeb129:
Config: {'init_qubits': True,
 'meas_level': <MeasLevel.CLASSIFIED: 2>,
 'memory': False,
 'memory_slots': 2,
 'n_qubits': 2,
 'parameter_binds': [],
 'shots': 1024,
 'use_measure_esp': True}
Header: {}
Experiments:

QASM Experiment:
Header:
{'clbit_labels': [['meas', 0], ['meas', 1]],
 'creg_sizes': [['meas', 2]],
 'global_phase': 0.0,
 'memory_slots': 2,
 'metadata': {},
 'n_qubits': 2,
 'name': 'circuit-0',
 'qreg_sizes': [['q', 2]],
 'qubit_labels': [['q', 0], ['q', 1]]}
Config:
{'memory_slots': 2, 'n_qubits': 2}

	Instruction: h
		qubits: [0]

	Instruction: cx
		qubits: [0, 1]

	Instruction: barrier
		qubits: [0, 1]

	Instruction: measure
		qubits: [0]
		memory: [0]

	Instruction: measure
		qubits: [1]
		memory: [1]

So all this is doing in practice is raising an exception at assemble() call time if use_measure_esp is enabled and the backend doesn't support it. But that arguably should be checked before the provider is calling to terra's assemble() function.

mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Jun 14, 2021
The changes made in Qiskit#6543 were unecessary and add ibm backend specific
options to common interfaces in terra. This commit reverts Qiskit#6543 so it
can be isolated to just the ibmq provider as the new options added in
it are not common to all providers and isolated to just ibm backends.
The changes made in Qiskit#6543 should have been done directly in the ibm
provider and no changes to terra are actually needed here. Terra already
provides all the functionality to enable the provider to add custom options
like this. A provider can advertise the new run time option via the
backend's `Options` object, expose the backend configuration option as
a custom attribute at construction time, and pass the setting from the
options to assemble so it gets set in the qobj config appropriately
without requiring any changes from terra. However, this isn't all
necessarily clear as the interface for `BackendV1` was primarily
inherited from `BaseBackend` which didn't have this level of flexibility
and doesn't make the distinction between required, optional, and custom
options and backend attributes super clear. This is something we can
hopefully address in the next version of the backend interface
`BackendV2` (ideas for this are being collected in Qiskit#5885).

This reverts commit 6d605a0.
@zachschoenfeld33
Copy link
Contributor Author

One thing I will say is it was far easier to debug and test from assemble than the provider. assemble has local unit tests, while the provider seems to require some server connections...

@mtreinish mtreinish added the Changelog: None Do not include in changelog label Jun 15, 2021
@zachschoenfeld33
Copy link
Contributor Author

This can be revoked in favor of Qiskit/qiskit-ibmq-provider#952.

mergify bot added a commit that referenced this pull request Jun 15, 2021
The changes made in #6543 were unecessary and add ibm backend specific
options to common interfaces in terra. This commit reverts #6543 so it
can be isolated to just the ibmq provider as the new options added in
it are not common to all providers and isolated to just ibm backends.
The changes made in #6543 should have been done directly in the ibm
provider and no changes to terra are actually needed here. Terra already
provides all the functionality to enable the provider to add custom options
like this. A provider can advertise the new run time option via the
backend's `Options` object, expose the backend configuration option as
a custom attribute at construction time, and pass the setting from the
options to assemble so it gets set in the qobj config appropriately
without requiring any changes from terra. However, this isn't all
necessarily clear as the interface for `BackendV1` was primarily
inherited from `BaseBackend` which didn't have this level of flexibility
and doesn't make the distinction between required, optional, and custom
options and backend attributes super clear. This is something we can
hopefully address in the next version of the backend interface
`BackendV2` (ideas for this are being collected in #5885).

This reverts commit 6d605a0.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@zachschoenfeld33 zachschoenfeld33 deleted the zas-esp-support branch June 16, 2021 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants