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

Projected Variational Quantum Dynamics #8304

Merged
merged 29 commits into from
Aug 4, 2022
Merged

Projected Variational Quantum Dynamics #8304

merged 29 commits into from
Aug 4, 2022

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Jul 6, 2022 •
edited
Loading

Summary

Implement the p-VQD algorithm for real time evolution from https://quantum-journal.org/papers/q-2021-07-28-512/.

This algorithm implement real time evolution by projecting a single Trotter step onto a variational form in each timestep.

Details and comments

Todo:

  • Release note
  • Tutorial accompanying this PR (will come in a separate PR to qiskit-tutorials)
  • Some more tests
  • Docs and imports

@Cryoris Cryoris added Changelog: New Feature Include in the "Added" section of the changelog mod: algorithms Related to the Algorithms module labels Jul 6, 2022
@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:

@coveralls
Copy link

coveralls commented Jul 6, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 2799260696

  • 180 of 181 (99.45%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 84.037%
Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/algorithms/evolvers/pvqd/pvqd.py 122 123 99.19%
Totals Coverage Status
Change from base Build 2799257026: 0.04%
Covered Lines: 56086
Relevant Lines: 66740
💛 - Coveralls

dlasecki
dlasecki suggested changes Jul 8, 2022
View reviewed changes
Copy link
Contributor

@dlasecki dlasecki left a comment

Choose a reason for hiding this comment

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

Left some feedback. Do you plan to make the pvqd class shorter by a decomposition?

qiskit/algorithms/evolvers/pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd.py Outdated Show resolved Hide resolved
test/python/algorithms/evolvers/test_pvqd.py Outdated Show resolved Hide resolved
test/python/algorithms/evolvers/test_pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd.py Outdated Show resolved Hide resolved
Cryoris added 3 commits July 8, 2022 11:44
@Cryoris
use gradient only if supported
c5b9c12
@Cryoris
polishing!
a361656 
- reno
- remove old pvqd file
- allow attributes to be None
- more tests
@Cryoris
fix algorithms import
35c7b28
@Cryoris Cryoris marked this pull request as ready for review July 14, 2022 12:03
@Cryoris Cryoris requested review from a team, manoelmarques and woodsp-ibm as code owners July 14, 2022 12:03
@dlasecki
Copy link
Contributor

Can we try to break down the pvqd.py class into smaller classes?

@Cryoris
Copy link
Contributor Author

Cryoris commented Jul 14, 2022

To reduce the filesize or what do you want to optimize? I could move the observable evaluator outside of the function or the result object into a new file. Or what did you have in mind?

@dlasecki
Copy link
Contributor

To reduce the filesize or what do you want to optimize? I could move the observable evaluator outside of the function or the result object into a new file. Or what did you have in mind?

Yes, I wanted to make the main class shorter and thus easier to read.
The class seems to have the following responsibilities now:

  1. evolve/step,
  2. evaluate loss,
  3. evaluate observables,
  4. check if gradient method possible.

If would say 2) and 3) are perhaps solid candidates for extraction.

@Cryoris
rm OrderedDict from unitary
8d3bcf5 
no idea why this unused import existed, that should be caught before this PR?
dlasecki
dlasecki previously approved these changes Jul 19, 2022
View reviewed changes
Copy link
Contributor

@dlasecki dlasecki left a comment

Choose a reason for hiding this comment

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

Good work! Glad to see another algorithm enriching the Evolvers Framework.

Zoufalc
Zoufalc previously approved these changes Jul 22, 2022
View reviewed changes
Copy link
Contributor

@Zoufalc Zoufalc left a comment

Choose a reason for hiding this comment

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

I just had some minor comments on terminology. Thanks for the addition to Qiskit. Looking forward to testing it :)

qiskit/algorithms/evolvers/evolution_problem.py Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd_result.py Outdated Show resolved Hide resolved
@Cryoris Cryoris dismissed stale reviews from Zoufalc and dlasecki via 712184c July 26, 2022 08:41
Zoufalc
Zoufalc approved these changes Jul 26, 2022
View reviewed changes
Copy link
Contributor

@Zoufalc Zoufalc left a comment

Choose a reason for hiding this comment

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

Thanks for incorporating the comments.

Zoufalc
Zoufalc previously approved these changes Jul 27, 2022
View reviewed changes
@Cryoris Cryoris mentioned this pull request Jul 27, 2022
Merged
woodsp-ibm
woodsp-ibm reviewed Jul 27, 2022
View reviewed changes
qiskit/algorithms/__init__.py Show resolved Hide resolved
qiskit/algorithms/evolvers/evolution_problem.py Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Outdated Show resolved Hide resolved
qiskit/algorithms/evolvers/pvqd/pvqd.py Outdated Show resolved Hide resolved
qiskit/extensions/unitary.py Outdated Show resolved Hide resolved
releasenotes/notes/project-dynamics-2f848a5f89655429.yaml Outdated Show resolved Hide resolved
Cryoris added 2 commits July 28, 2022 14:16
@Cryoris
include comments from review
ba088e4 
- support MatrixOp
- default for timestep
- update reno with refs
- test step and get_loss
@Cryoris
Merge branch 'main' into pvqd
0e4f8ee
@mergify mergify bot merged commit 3705798 into Qiskit:main Aug 4, 2022
@Cryoris Cryoris deleted the pvqd branch August 5, 2022 08:24
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Jun 27, 2023
@Cryoris @mergify
Projected Variational Quantum Dynamics (Qiskit#8304)
9befaa6 
* pvqd dradt

* update as theta + difference

* black

* refactor test

* update to new time evo framework

* add gradients

* use gradient only if supported

* polishing!

- reno
- remove old pvqd file
- allow attributes to be None
- more tests

* fix algorithms import

* changes from code review

* add more tests for  different ops

* refactor PVQD to multiple files

* remove todo

* comments from review

* rm OrderedDict from unitary

no idea why this unused import existed, that should be caught before this PR?

* changes from code review

* remove function to attach intial states

* include comments from review

- support MatrixOp
- default for timestep
- update reno with refs
- test step and get_loss

* make only quantum instance and optimizer optional, and use num_timesteps

* fix docs

* add comment why Optimizer is optional

* use class attributes to document mutable attrs

* rm duplicate quantum_instance doc

* fix attributes docs

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-algorithms-test that referenced this pull request Jul 17, 2023
@Cryoris @mergify
Projected Variational Quantum Dynamics (Qiskit/qiskit#8304)
adc48b5 
* pvqd dradt

* update as theta + difference

* black

* refactor test

* update to new time evo framework

* add gradients

* use gradient only if supported

* polishing!

- reno
- remove old pvqd file
- allow attributes to be None
- more tests

* fix algorithms import

* changes from code review

* add more tests for  different ops

* refactor PVQD to multiple files

* remove todo

* comments from review

* rm OrderedDict from unitary

no idea why this unused import existed, that should be caught before this PR?

* changes from code review

* remove function to attach intial states

* include comments from review

- support MatrixOp
- default for timestep
- update reno with refs
- test step and get_loss

* make only quantum instance and optimizer optional, and use num_timesteps

* fix docs

* add comment why Optimizer is optional

* use class attributes to document mutable attrs

* rm duplicate quantum_instance doc

* fix attributes docs

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

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

@Zoufalc Zoufalc Zoufalc left review comments

@dlasecki dlasecki dlasecki left review comments

@manoelmarques manoelmarques Awaiting requested review from manoelmarques

Assignees
No one assigned
Labels
Changelog: New Feature Include in the "Added" section of the changelog mod: algorithms Related to the Algorithms module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Cryoris @qiskit-bot @coveralls @dlasecki @woodsp-ibm @Zoufalc