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 logic to return channels to FakeBackendV2 #8444

Merged

Conversation

nkanazawa1989
Copy link
Contributor

@nkanazawa1989 nkanazawa1989 commented Aug 3, 2022

Summary

This PR adds a logic to FakeBackendV2 that parses the reported channel mapping and returns the pulse channels with method call, such as .drive_channel(qubit). Because the corresponding V1 backend returns the channels, this is kind of a bug due to missing logic.

Details and comments

Fix #7832

@nkanazawa1989 nkanazawa1989 added Changelog: Bugfix Include in the "Fixed" section of the changelog mod: fake_provider Related to the fake_provider module and fake backends labels Aug 3, 2022
@nkanazawa1989 nkanazawa1989 requested review from a team and jyu00 as code owners August 3, 2022 13:01
@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

@mtreinish mtreinish added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Aug 3, 2022
@mtreinish
Copy link
Member

This has partial overlap with #8318 although this covers more of the optional attributes in the BackendV2 interface. @gadial are you ok with closing #8318 in favor of this? Or do you want to take @nkanazawa1989's additions into #8318 too

@nkanazawa1989
Copy link
Contributor Author

The question is if we can instantiate PulseBackendConfiguration with V2. Seems like this approach is overkill to just parse the channels mapping, i.e. PulseBackendConfiguration._parse_channels.

@gadial
Copy link
Contributor

gadial commented Aug 3, 2022

This has partial overlap with #8318 although this covers more of the optional attributes in the BackendV2 interface. @gadial are you ok with closing #8318 in favor of this? Or do you want to take @nkanazawa1989's additions into #8318 too

I think Naoki's PR is better since it is more focused; I intend to close #8318 .

@coveralls
Copy link

coveralls commented Aug 3, 2022

Pull Request Test Coverage Report for Build 2907173820

  • 38 of 43 (88.37%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.004%) to 84.042%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/providers/fake_provider/fake_backend.py 38 43 88.37%
Files with Coverage Reduction New Missed Lines %
qiskit/extensions/quantum_initializer/squ.py 2 79.78%
Totals Coverage Status
Change from base Build 2907171101: 0.004%
Covered Lines: 56260
Relevant Lines: 66943

💛 - Coveralls

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

LGTM, one inline comment/question but not a blocker.

"measure": pulse.MeasureChannel,
"control": pulse.ControlChannel,
}
identifier_pattern = re.compile(r"\D+(?P<index>\d+)")
Copy link
Member

Choose a reason for hiding this comment

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

Not that regex time here will be a big deal but do you think it'll be worthwhile to move this to a class level variable so we don't have to have the compile time on each instantiation of a FakeBackendV2?

@mergify mergify bot merged commit 3f9edfc into Qiskit:main Aug 22, 2022
mergify bot pushed a commit that referenced this pull request Aug 22, 2022
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 3f9edfc)
mergify bot added a commit that referenced this pull request Aug 22, 2022
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 3f9edfc)

Co-authored-by: Naoki Kanazawa <[email protected]>
@nkanazawa1989 nkanazawa1989 deleted the fix/missing_channels_implementation branch November 25, 2022 02:55
ElePT pushed a commit to ElePT/qiskit-ibm-provider that referenced this pull request Oct 4, 2023
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-ibm-runtime that referenced this pull request Oct 10, 2023
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Oct 12, 2023
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-ibm-runtime that referenced this pull request Dec 8, 2023
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog mod: fake_provider Related to the fake_provider module and fake backends stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pulse channels to FakeBackendV2
5 participants