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 RZX builder rescaled amplitude #8031

Merged
merged 3 commits into from
May 11, 2022

Conversation

nkanazawa1989
Copy link
Contributor

Summary

This PR fixes a bug that RZX builder passes return rescaled pulse amplitude in the parameter expression type that results in this pulse visualization. @nbronn

image

Details and comments

theta is a value that is provided by circuit node, which is not typecasted to float after the value assignment. Thus computed CR amp becomes parameter expression without explicit typecast.

@nkanazawa1989 nkanazawa1989 requested review from a team, eggerdj and DanPuzzuoli as code owners May 9, 2022 19:20
@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 nkanazawa1989 added the Changelog: Bugfix Include in the "Fixed" section of the changelog label May 9, 2022
@mtreinish mtreinish added the stable backport potential The bug might be minimal and/or import enough to be port to stable label May 9, 2022
@coveralls
Copy link

coveralls commented May 9, 2022

Pull Request Test Coverage Report for Build 2309370174

  • 4 of 6 (66.67%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.003%) to 84.379%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/transpiler/passes/calibration/builders.py 4 6 66.67%
Totals Coverage Status
Change from base Build 2307415935: 0.003%
Covered Lines: 54502
Relevant Lines: 64592

💛 - Coveralls

@jakelishman jakelishman added this to the 0.20.2 milestone May 11, 2022
@jakelishman
Copy link
Member

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented May 11, 2022

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@mergify mergify bot merged commit 829f232 into Qiskit:main May 11, 2022
mergify bot pushed a commit that referenced this pull request May 11, 2022
* Fix RZX builder rescale amplitude bug

* Update releasenotes/notes/fix-rzx-builder-pulse-amp-ba5c876ddea17c41.yaml

Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 829f232)
mergify bot added a commit that referenced this pull request May 11, 2022
* Fix RZX builder rescale amplitude bug

* Update releasenotes/notes/fix-rzx-builder-pulse-amp-ba5c876ddea17c41.yaml

Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 829f232)

Co-authored-by: Naoki Kanazawa <nkanazawa1989@gmail.com>
ajavadia pushed a commit to ajavadia/qiskit that referenced this pull request May 31, 2022
* Fix RZX builder rescale amplitude bug

* Update releasenotes/notes/fix-rzx-builder-pulse-amp-ba5c876ddea17c41.yaml

Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@nkanazawa1989 nkanazawa1989 deleted the fix/rzx-builder-amp-typecast branch November 25, 2022 02:52
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 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.

5 participants