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 the bug when doing schedule with measurements for BackendV2 #10299

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

chuanqixu
Copy link

Summary

Currently, it does not support BackendV2 for scheduling.
I check backend.instruction_schedule_map, and it seems that the difference is that BackendV1 has the simultaneous measurement operation for all qubits, while BackendV2 does not have such an operation.

Details and comments

More details can be found in #10298 .

I solved this with simple changes.
Only added one argument for ScheduleConfig.

@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jun 16, 2023
@CLAassistant
Copy link

CLAassistant commented Jun 16, 2023

CLA assistant check
All committers have signed the CLA.

@chuanqixu chuanqixu marked this pull request as ready for review June 17, 2023 14:29
@chuanqixu chuanqixu requested review from a team, eggerdj and wshanks as code owners June 17, 2023 14:29
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core
  • @nkanazawa1989

@chuanqixu chuanqixu marked this pull request as draft June 17, 2023 14:30
@TsafrirA
Copy link
Collaborator

Note the implementation proposed in #10285 .

@chuanqixu
Copy link
Author

Note the implementation proposed in #10285 .

Thanks for pointing this out!

I found and mentioned this PR in my issue #10298 before I created this PR, so I only make this PR to be a draft.
Only provide another easy way to fix the issue at this point, but this requires changing the interface.

@to24toro
Copy link
Contributor

Thanks, @chuanqixu .
We feel like that we would like to deprecate ScheduleConfig in near future and its role would be replaced with Target.

@chuanqixu
Copy link
Author

@to24toro Thank you for the information!
Your solution and contribution are great!

I checked the code and discussions regarding this bug before creating this PR and was aware of the decision to replace ScheduleConfig with Target.
This should be a good choice since these two classes are a little redundant.

As I mentioned in #10298, the bug is due to the current scheduling process when calling qiskit.pulse.measure (in qiskit/pulse/macro.py).
This function should be correctly written, which uses the argument backend to decide the backend version and then choose to return _measure_v1 or _measure_v2.

However, the scheduling bug is due to that no backend is passed to this argument, and thus it always uses the default value None. So even for V2 Backend, it still returns _measure_v1` and thus leads to the bug.

I checked all references in the Qiskit source code, and it seems that there is no place passing backend to qiskit.pulse.measure.
I also checked the solution in #10285. Now it uses both the backend and target to decide the backend version.
I can get the writers' idea that they may want to provide two approaches for giving pulse information, and this kind of design is widely used in Qiskit.

I agree with the solution in #10285.
This PR is to provide a quick solution for people who need to use schedule with V2 Backend before the release of the next version of Qiskit.
Feel free to close this PR and my issue #10298 for now, and I will close them when the new version of Qiskit is out.

@nkanazawa1989
Copy link
Contributor

@to24toro Can we close the PR in favor of #10564 ?

@to24toro
Copy link
Contributor

to24toro commented Aug 11, 2023

@chuanqixu
I am finalizing the detail about scheduling with backendV2 in this PR.
You can check the further discussion in this PR.

@nkanazawa1989
Yes. We can close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community PR PRs from contributors that are not 'members' of the Qiskit repo
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

6 participants