Skip to content

Time Evolution Framework with primitives.#8681

Merged
mergify[bot] merged 31 commits into
Qiskit:mainfrom
dlasecki:evolution-framework-primitives
Sep 13, 2022
Merged

Time Evolution Framework with primitives.#8681
mergify[bot] merged 31 commits into
Qiskit:mainfrom
dlasecki:evolution-framework-primitives

Conversation

@dlasecki
Copy link
Copy Markdown
Contributor

@dlasecki dlasecki commented Sep 5, 2022

Summary

This PR moves time evolution interfaces and classes to a new location where primitive-enabled time evolution algorithms will reside. It also limits the use of opflow objects.

Closes: TODO

Details and comments

@qiskit-bot
Copy link
Copy Markdown
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
Copy Markdown

coveralls commented Sep 5, 2022

Pull Request Test Coverage Report for Build 3048002047

  • 79 of 81 (97.53%) changed or added relevant lines in 11 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 84.339%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/algorithms/time_evolvers/imaginary_time_evolver.py 7 8 87.5%
qiskit/algorithms/time_evolvers/real_time_evolver.py 7 8 87.5%
Totals Coverage Status
Change from base Build 3047917829: 0.02%
Covered Lines: 57960
Relevant Lines: 68723

💛 - Coveralls

Copy link
Copy Markdown
Collaborator

@Cryoris Cryoris left a comment

Choose a reason for hiding this comment

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

Some minor comments otherwise LGTM 👍🏻

Comment thread qiskit/algorithms/time_evolvers/evolution_problem.py Outdated
Comment thread qiskit/algorithms/time_evolvers/evolution_problem.py Outdated
Comment thread qiskit/algorithms/time_evolvers/evolution_problem.py
Comment thread qiskit/algorithms/time_evolvers/evolution_result.py Outdated
@woodsp-ibm
Copy link
Copy Markdown
Member

woodsp-ibm commented Sep 6, 2022

This PR moves time evolution interfaces and classes to a new location

This is creating new interfaces here based on a copy of the originals right (not moving them perse). The originals, in evolvers, of these 4 files need to be pending deprecated - along with trotterization trotter_qrte as these were the only files released (basically what was added by #7411) that are presently in evolvers in main.

Should we do trotter_qrte in this same PR since that would span all the code which has been released (code added by VarQTE and PVQD, since it has not yet been released, can be moved).

At this point I think we should add a time_evolvers link to algorithms init so as to start on the documentation. We may need to update the init file here so that they get documented too. Since we need to maintain evolvers they cannot collide for now what was already imported/documented directly in algorithms rather a user will explicitly have to import from time_evolvers until such time as the originals are removed and we can allow import back at the algorithms level of these new ones too. Though arguably if we also added Time to the class names we could have them at algorithms level too - e.g EvolutionProblem -> TimeEvolutionProblem, RealEvolver -> RealTimeEvolver etc. Naming thus would arguably fit better with having these as time evolvers.

Comment thread qiskit/algorithms/time_evolvers/evolution_problem.py Outdated
Comment thread qiskit/algorithms/time_evolvers/evolution_problem.py
Comment thread releasenotes/notes/evolution-framework-primitives-c86779b5d0dffd25.yaml Outdated
@woodsp-ibm
Copy link
Copy Markdown
Member

@dlasecki Did you have any feedback to this comment from above

At this point I think we should add a time_evolvers link to algorithms init so as to start on the documentation. We may need to update the init file here so that they get documented too. Since we need to maintain evolvers they cannot collide for now what was already imported/documented directly in algorithms rather a user will explicitly have to import from time_evolvers until such time as the originals are removed and we can allow import back at the algorithms level of these new ones too. Though arguably if we also added Time to the class names we could have them at algorithms level too - e.g EvolutionProblem -> TimeEvolutionProblem, RealEvolver -> RealTimeEvolver etc. Naming thus would arguably fit better with having these as time evolvers.

@manoelmarques manoelmarques force-pushed the evolution-framework-primitives branch from c8170ca to 387ce78 Compare September 8, 2022 15:14
@dlasecki
Copy link
Copy Markdown
Contributor Author

dlasecki commented Sep 8, 2022

@dlasecki Did you have any feedback to this comment from above

At this point I think we should add a time_evolvers link to algorithms init so as to start on the documentation. We may need to update the init file here so that they get documented too. Since we need to maintain evolvers they cannot collide for now what was already imported/documented directly in algorithms rather a user will explicitly have to import from time_evolvers until such time as the originals are removed and we can allow import back at the algorithms level of these new ones too. Though arguably if we also added Time to the class names we could have them at algorithms level too - e.g EvolutionProblem -> TimeEvolutionProblem, RealEvolver -> RealTimeEvolver etc. Naming thus would arguably fit better with having these as time evolvers.

Hello, I did not think much about it beforehand but I liked your idea about adding Time to class names. It will be more consistent with the package, more explanatory and a clearer differentiator from old classes. I suppose we can accept slightly longer names due to this change. I went ahead and updated classes according to your idea, including linking them to the algorithms init file which should solve the missing documentation.

@dlasecki dlasecki marked this pull request as ready for review September 9, 2022 07:42
@dlasecki dlasecki requested review from a team and manoelmarques as code owners September 9, 2022 07:42
Comment thread qiskit/algorithms/__init__.py
Comment thread qiskit/algorithms/evolvers/evolution_problem.py Outdated
Comment thread qiskit/algorithms/evolvers/pvqd/pvqd.py
Comment thread qiskit/algorithms/time_evolvers/time_evolution_problem.py Outdated
Comment thread qiskit/algorithms/time_evolvers/time_evolution_problem.py Outdated
Comment thread qiskit/algorithms/time_evolvers/time_evolution_problem.py Outdated
@woodsp-ibm woodsp-ibm added this to the 0.22 milestone Sep 12, 2022
@manoelmarques manoelmarques force-pushed the evolution-framework-primitives branch from afbdaec to 4795b45 Compare September 12, 2022 17:37
@woodsp-ibm woodsp-ibm added Changelog: Deprecated Add a "Deprecated" entry in the GitHub Release changelog. Changelog: Added Add an "Added" entry in the GitHub Release changelog. labels Sep 12, 2022
@mergify mergify Bot merged commit b35b18a into Qiskit:main Sep 13, 2022
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Jun 27, 2023
* Implemented observables_evaluator.py with primitives.

* Added evolvers problems and interfaces to time_evolvers package.

* Mostly updated trotter_qrte.py to use primitives.

* Added observables_evaluator.py that uses primitives.

* Added observables_evaluator.py that uses primitives.

* Updated trotter_qrte.py to use primitives.

* Updated imports

* Updated typehints and limited use of opflow.

* Updated typehints and limited use of opflow.

* Removed files out of scope for this PR.

* Added annotations import.

* Applied some CR comments.

* Added reno.

* Accepting Statevector.

* Added attributes docs.

* Add pending deprecation for evolvers

* Renamed classes and linked to algorithms init.

* fix docstring

* Improved reno.

* Code refactoring.

* Black fix.

* Applied CR comments.

* Add deprecation msg to evolvers package

Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-algorithms-test that referenced this pull request Jul 17, 2023
* Implemented observables_evaluator.py with primitives.

* Added evolvers problems and interfaces to time_evolvers package.

* Mostly updated trotter_qrte.py to use primitives.

* Added observables_evaluator.py that uses primitives.

* Added observables_evaluator.py that uses primitives.

* Updated trotter_qrte.py to use primitives.

* Updated imports

* Updated typehints and limited use of opflow.

* Updated typehints and limited use of opflow.

* Removed files out of scope for this PR.

* Added annotations import.

* Applied some CR comments.

* Added reno.

* Accepting Statevector.

* Added attributes docs.

* Add pending deprecation for evolvers

* Renamed classes and linked to algorithms init.

* fix docstring

* Improved reno.

* Code refactoring.

* Black fix.

* Applied CR comments.

* Add deprecation msg to evolvers package

Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: Added Add an "Added" entry in the GitHub Release changelog. Changelog: Deprecated Add a "Deprecated" entry in the GitHub Release changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants