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

Remove workarounds for passing run options to sampler #1498

Merged
merged 2 commits into from
Dec 10, 2024

Conversation

wshanks
Copy link
Collaborator

@wshanks wshanks commented Dec 8, 2024

This runtime patching code was added along with the initial support for sampler execution because the path through
qiskit_ibm_runtime.SamplerV2 through to BackendSamplerV2 did not carry through options for level 1 measurements or noise models. This path is used in the tests (while execution on physical backends with SamplerV2 does not use this path). Now with Qiskit 1.3 and qiskit-ibm-runtime 0.34, this patching is no longer needed. Remaining warnings filtering was moved to the calibrations tutorial where it was needed.

This runtime patching code was added along with the initial support for
sampler execution because the path through
`qiskit_ibm_runtime.SamplerV2` through to `BackendSamplerV2` did not
carry through options for level 1 measurements or noise models. This
path is used in the tests (while execution on physical backends with
`SamplerV2` does not use this path). Now with Qiskit 1.3 and
qiskit-ibm-runtime 0.34, this patching is no longer needed. Remaining
warnings filtering was moved to the calibrations tutorial where it was
needed.
@wshanks wshanks added the backport stable potential The issue or PR might be minimal and/or import enough to backport to stable label Dec 8, 2024
Copy link
Collaborator

@yaelbh yaelbh left a comment

Choose a reason for hiding this comment

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

Can you please explain some more, what was the situation before the recent versions of qiskit and qiskit-ibm-runtime, and what has changed. How did physical backends cope with measurement level 1?

requirements-dev.txt Outdated Show resolved Hide resolved
requirements-dev.txt Outdated Show resolved Hide resolved
Co-authored-by: Yael Ben-Haim <yaelbh@il.ibm.com>
@wshanks
Copy link
Collaborator Author

wshanks commented Dec 9, 2024

In #1470, the default behavior of BaseExperiment.run was changed to create a qiskit_ibm_runtime.SamplerV2 instance with the backend object passed to it and then use it to run the experiment's circuits (with other execution paths possible, either by passing a different sampler or using backend_run=True to use the old path of calling backend.run). For qiskit_ibm_runtime.IBMBackend instances (the physical backends), measurement level 1 was already supported and execution through the sampler worked okay. When using a different backend, qiskit_ibm_runtime.SamplerV2 falls back to using qiskit.primitives.BackendSamplerV2 which calls backend.run. The path of Qiskit Experiments run options -> SamplerV2 options -> BackendSamplerV2 options -> backend.run was not fully connected for getting options like meas_level, meas_return, and noise_model through to the backend. Additionally, BackendSamplerV2 only had support for translating level 2 results back from the backend.run format to the primitive result format.

To keep our tests working for #1470, I added the code removed in this PR which monkey patches the code related to SamplerV2 and BackendSamplerV2 to pass the options through and convert the results back. This monkey patching (in the removed qiskit_experiments/test/patching.py file) was fragile and intended to be temporary. It copied the necessary functions from the other packages with modifications and then overwrote those functions in the modules they came from. The modifications changed a little bit in code review but they were basically the changes that were accepted upstream in Qiskit/qiskit#13357 and Qiskit/qiskit-ibm-runtime#1990.

Copy link
Collaborator

@yaelbh yaelbh left a comment

Choose a reason for hiding this comment

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

Approved (I can't click the Approve button for some reason)

@yaelbh yaelbh added this pull request to the merge queue Dec 10, 2024
Merged via the queue into qiskit-community:main with commit 9bf2848 Dec 10, 2024
11 checks passed
mergify bot pushed a commit that referenced this pull request Dec 10, 2024
This runtime patching code was added along with the initial support for
sampler execution because the path through
`qiskit_ibm_runtime.SamplerV2` through to `BackendSamplerV2` did not
carry through options for level 1 measurements or noise models. This
path is used in the tests (while execution on physical backends with
`SamplerV2` does not use this path). Now with Qiskit 1.3 and
qiskit-ibm-runtime 0.34, this patching is no longer needed. Remaining
warnings filtering was moved to the calibrations tutorial where it was
needed.

---------

Co-authored-by: Yael Ben-Haim <yaelbh@il.ibm.com>
(cherry picked from commit 9bf2848)
wshanks added a commit that referenced this pull request Dec 10, 2024
#1500)

This runtime patching code was added along with the initial support for
sampler execution because the path through
`qiskit_ibm_runtime.SamplerV2` through to `BackendSamplerV2` did not
carry through options for level 1 measurements or noise models. This
path is used in the tests (while execution on physical backends with
`SamplerV2` does not use this path). Now with Qiskit 1.3 and
qiskit-ibm-runtime 0.34, this patching is no longer needed. Remaining
warnings filtering was moved to the calibrations tutorial where it was
needed.<hr>This is an automatic backport of pull request #1498 done by
[Mergify](https://mergify.com).

Co-authored-by: Will Shanks <willshanks@us.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport stable potential The issue or PR might be minimal and/or import enough to backport to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants