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.

Sign up for GitHub

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

Jump to bottom

Efficient classical calculation of expectation gradients #9287

Merged
merged 19 commits into from
Jan 17, 2023
Merged

Efficient classical calculation of expectation gradients #9287

merged 19 commits into from
Jan 17, 2023

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Dec 14, 2022 •
edited
Loading

Summary

Closes #9037.

Details and comments

This PR also adds derivative_type to the interface of the base estimator gradient to make it accessible throughout the gradients interface.

@Cryoris Cryoris requested review from a team, manoelmarques and woodsp-ibm as code owners December 14, 2022 16:02
@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:

@Cryoris
Copy link
Contributor Author

Cryoris commented Dec 14, 2022

Also tagging @a-matsuo and @ElePT for review.

@Cryoris Cryoris added Changelog: New Feature Include in the "Added" section of the changelog mod: algorithms Related to the Algorithms module labels Dec 14, 2022
@Cryoris Cryoris added this to the 0.23.0 milestone Dec 14, 2022
@coveralls
Copy link

coveralls commented Dec 14, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 3943729740

  • 278 of 316 (87.97%) changed or added relevant lines in 10 files are covered.
  • 7 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.02%) to 84.835%
Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/algorithms/gradients/reverse_gradient/split_circuits.py 24 25 96.0%
qiskit/algorithms/gradients/reverse_gradient/reverse_gradient.py 78 80 97.5%
qiskit/algorithms/gradients/reverse_gradient/reverse_qgt.py 105 109 96.33%
qiskit/algorithms/gradients/reverse_gradient/derive_circuit.py 43 74 58.11%
Files with Coverage Reduction New Missed Lines %
qiskit/algorithms/gradients/lin_comb_estimator_gradient.py 1 95.83%
qiskit/pulse/library/waveform.py 3 91.67%
src/vf2_layout.rs 3 94.74%
Totals Coverage Status
Change from base Build 3943571132: -0.02%
Covered Lines: 65884
Relevant Lines: 77661
💛 - Coveralls

ElePT
ElePT reviewed Dec 15, 2022
View reviewed changes
Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

Hi Julien! Thanks for your PR! I went through it and left a few comments.

qiskit/algorithms/gradients/reverse_gradient/bind.py Outdated Show resolved Hide resolved
qiskit/algorithms/gradients/reverse_gradient/bind.py Outdated Show resolved Hide resolved
qiskit/algorithms/gradients/reverse_gradient/bind.py Outdated Show resolved Hide resolved
qiskit/algorithms/gradients/reverse_gradient/derive_circuit.py Outdated Show resolved Hide resolved
qiskit/algorithms/gradients/reverse_gradient/derive_circuit.py Outdated Show resolved Hide resolved
qiskit/algorithms/gradients/reverse_gradient/reverse_gradient.py Outdated Show resolved Hide resolved
qiskit/algorithms/gradients/reverse_gradient/reverse_gradient.py Outdated Show resolved Hide resolved
qiskit/algorithms/gradients/reverse_gradient/reverse_gradient.py Outdated Show resolved Hide resolved
qiskit/algorithms/gradients/reverse_gradient/reverse_gradient.py Show resolved Hide resolved
releasenotes/notes/turbo-gradients-5bebc6e665b900b2.yaml Outdated Show resolved Hide resolved
@Cryoris Cryoris mentioned this pull request Jan 10, 2023
Closed
ElePT
ElePT reviewed Jan 17, 2023
View reviewed changes
Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

From my superficial understanding of what these gradients are supposed to do, the code looks good to me. So much so that I can only point out the copyright change.

qiskit/algorithms/gradients/reverse_gradient/reverse_qgt.py Outdated Show resolved Hide resolved
@Cryoris @woodsp-ibm
Update copyrights
c7954bf 
Co-authored-by: Steve Wood <[email protected]>
@mergify mergify bot merged commit 0cf755f into Qiskit:main Jan 17, 2023
@Cryoris Cryoris deleted the turbo-gradients branch February 22, 2023 10:08
ElePT added a commit to ElePT/qiskit that referenced this pull request Jun 27, 2023
@Cryoris @ElePT
@woodsp-ibm @mergify
Efficient classical calculation of expectation gradients (Qiskit#9287)
44ba8c5 
* ckassically efficient gradients

* cleanup & reno

* Apply suggestions from code review

Co-authored-by: ElePT <[email protected]>

* Remove support for Parameter = None

* Complete the docs

* QGT v0

* fix LCU tests

* final fixes

* Update after QGT merge

* print which parameter is not in the circuit

* Fix copyright

Co-authored-by: ElePT <[email protected]>

* ``None`` is not actually supported

* only setter of derivative_type in LCU/Rev

* Update copyrights

Co-authored-by: Steve Wood <[email protected]>

Co-authored-by: ElePT <[email protected]>
Co-authored-by: Steve Wood <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT added a commit to ElePT/qiskit-algorithms-test that referenced this pull request Jul 17, 2023
@Cryoris @ElePT
@woodsp-ibm @mergify
Efficient classical calculation of expectation gradients (Qiskit/qisk…
ce917f2 
…it#9287)

* ckassically efficient gradients

* cleanup & reno

* Apply suggestions from code review

Co-authored-by: ElePT <[email protected]>

* Remove support for Parameter = None

* Complete the docs

* QGT v0

* fix LCU tests

* final fixes

* Update after QGT merge

* print which parameter is not in the circuit

* Fix copyright

Co-authored-by: ElePT <[email protected]>

* ``None`` is not actually supported

* only setter of derivative_type in LCU/Rev

* Update copyrights

Co-authored-by: Steve Wood <[email protected]>

Co-authored-by: ElePT <[email protected]>
Co-authored-by: Steve Wood <[email protected]>
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
Reviewers

@ElePT ElePT ElePT left review comments

@woodsp-ibm woodsp-ibm woodsp-ibm approved these changes

@manoelmarques manoelmarques Awaiting requested review from manoelmarques

Assignees

@ElePT ElePT

Labels
Changelog: New Feature Include in the "Added" section of the changelog mod: algorithms Related to the Algorithms module priority: high
Projects
None yet
Milestone
0.23.0
Development

Successfully merging this pull request may close these issues.

Classically efficient gradients and QFI
6 participants
@Cryoris @qiskit-bot @coveralls @woodsp-ibm @ElePT @mtreinish