-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Implement quantum phase estimation algorithms #5642
Conversation
c045a8e
to
c909f02
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, I left a few comments and will try to play with it later 🙂
qiskit/algorithms/phase_estimators/hamiltonian_phase_estimation.py
Outdated
Show resolved
Hide resolved
qiskit/algorithms/phase_estimators/hamiltonian_phase_estimation.py
Outdated
Show resolved
Hide resolved
qiskit/algorithms/phase_estimators/hamiltonian_phase_estimation.py
Outdated
Show resolved
Hide resolved
qiskit/algorithms/phase_estimators/hamiltonian_phase_estimation.py
Outdated
Show resolved
Hide resolved
qiskit/algorithms/phase_estimators/hamiltonian_phase_estimation.py
Outdated
Show resolved
Hide resolved
…n.py Co-authored-by: Julien Gacon <[email protected]>
Replace default arg with None, and then set the desired default in the body of the function.
Move all common code out of _run for HamiltonianPhaseEstimation and PhaseEstimation, and rename _run to _result, because it only creates the result class.
* An unknown change in the past three weeks caused tests to fail with the default filter threshold of 0 (no filtering). I added a small cutoff so that the tests pass again. * Fix linter complaints
Yes, I should update that note. I think some of the underlying problems might remain (outside of algorithms/opflow). But, by treating the identity term separately, I avoid triggering them.
I think the consensus was not to implement the MinimumEigenSolver-like IQPE, since it is much more involved and experimental. But, yes it looks like the plain iqpe implementation has gone missing! Maybe I did not copy it over from the older aqua PR. @Cryoris also requested some other small changes on a different channel. Those remain to be finished and pushed to this branch. |
* be explicit about which evolution is used * combine tests using ddt * add some linebreaks
…nto phase-estimation
…nto phase-estimation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. In a follow up we can try to add the Minimum Eigensolver interfaces in time for the release. 👍🏻
* Add PhaseEstimator and related classes * Fix linter complaints * Update qiskit/algorithms/phase_estimators/hamiltonian_phase_estimation.py Co-authored-by: Julien Gacon <[email protected]> * Fix indentation in doc string * Fix more indentation in doc string * Comment on reasons for removing identity term from Hamiltonian * Replace mistaken description "water" by "H2" * Add default phase shift of 0.0 for identity term * Remove function call from default arg Replace default arg with None, and then set the desired default in the body of the function. * Refactor _run and rename to _result Move all common code out of _run for HamiltonianPhaseEstimation and PhaseEstimation, and rename _run to _result, because it only creates the result class. * Move some inputs and calculation to estimate method * Add cutoff to filter in test_from_bound * An unknown change in the past three weeks caused tests to fail with the default filter threshold of 0 (no filtering). I added a small cutoff so that the tests pass again. * Fix linter complaints * Fix linter complaints * Allow PauliSumOp as input Hamiltonian * Make suggested changes * Move module method to classmethod * Fix indentation * Refactor scale_phase scale_phases * Make HamiltonianPhaseEstimation have a hasa rather than isa use of PE * Remove all computation from PhaseEstimator * Remove ability to set number of computation qubits in PhaseEstimator.estimate() * update to current algorithm paradigm + small fixes * make algorithms stateless (store no problem information) * fix lint and docstrings * fix cyclic imports * Use PauliTrotterEvolution by default in HamiltonianPhaseEstimation * Make HamiltonianPhaseEstmation take a StateFn for state preparation * Add method most_likely_phase to HamiltonianPhaseEstimation * Allow PauliOp as input unitary to HPE * Allow MatrixOp as input Hermitian op to HPE * Fix linter complaints * fix evolution = None case * refactor tests slightly * be explicit about which evolution is used * combine tests using ddt * add some linebreaks * fix import order * Improve docstrings for PE and HPE * attempt to fix sphinx * attempt to fix sphinx #2 * attempt no. 3 to fix sphinx * layout, don't use function calls in defaults * trailing comma Co-authored-by: Julien Gacon <[email protected]>
* Add PhaseEstimator and related classes * Fix linter complaints * Update qiskit/algorithms/phase_estimators/hamiltonian_phase_estimation.py Co-authored-by: Julien Gacon <[email protected]> * Fix indentation in doc string * Fix more indentation in doc string * Comment on reasons for removing identity term from Hamiltonian * Replace mistaken description "water" by "H2" * Add default phase shift of 0.0 for identity term * Remove function call from default arg Replace default arg with None, and then set the desired default in the body of the function. * Refactor _run and rename to _result Move all common code out of _run for HamiltonianPhaseEstimation and PhaseEstimation, and rename _run to _result, because it only creates the result class. * Move some inputs and calculation to estimate method * Add cutoff to filter in test_from_bound * An unknown change in the past three weeks caused tests to fail with the default filter threshold of 0 (no filtering). I added a small cutoff so that the tests pass again. * Fix linter complaints * Fix linter complaints * Allow PauliSumOp as input Hamiltonian * Make suggested changes * Move module method to classmethod * Fix indentation * Refactor scale_phase scale_phases * Make HamiltonianPhaseEstimation have a hasa rather than isa use of PE * Remove all computation from PhaseEstimator * Remove ability to set number of computation qubits in PhaseEstimator.estimate() * update to current algorithm paradigm + small fixes * make algorithms stateless (store no problem information) * fix lint and docstrings * fix cyclic imports * Use PauliTrotterEvolution by default in HamiltonianPhaseEstimation * Make HamiltonianPhaseEstmation take a StateFn for state preparation * Add method most_likely_phase to HamiltonianPhaseEstimation * Allow PauliOp as input unitary to HPE * Allow MatrixOp as input Hermitian op to HPE * Fix linter complaints * fix evolution = None case * refactor tests slightly * be explicit about which evolution is used * combine tests using ddt * add some linebreaks * fix import order * Improve docstrings for PE and HPE * attempt to fix sphinx * attempt to fix sphinx Qiskit/qiskit#2 * attempt no. 3 to fix sphinx * layout, don't use function calls in defaults * trailing comma Co-authored-by: Julien Gacon <[email protected]>
This PR implements QPE, both parallel (i.e. as in Nielsen and Chuang) and iterative.
This PR is marked WIP mostly because the parallel algorithms fail for some inputs, most likely because of bugs elsewhere.EDIT: This has been fixed.
This PR was originally to aqua qiskit-community/qiskit-aqua#1245 . Because of the larger repository reorganization, that PR was closed and the code was copied to a branch in terra.