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

Rewrite Amplitude Estimators with Primitives #8656

Merged
merged 35 commits into from
Sep 24, 2022
Merged

Conversation

manoelmarques
Copy link
Member

Summary

closes #8479
closes #8480
closes #8481
closes #8482

Details and comments

@manoelmarques manoelmarques added the mod: algorithms Related to the Algorithms module label Aug 31, 2022
@manoelmarques manoelmarques self-assigned this Aug 31, 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:

@manoelmarques manoelmarques changed the title Rewrite Amplitude Estimators with Primitives [WIP] Rewrite Amplitude Estimators with Primitives Aug 31, 2022
@coveralls
Copy link

coveralls commented Aug 31, 2022

Pull Request Test Coverage Report for Build 3116197228

  • 226 of 248 (91.13%) changed or added relevant lines in 7 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.1%) to 84.416%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/algorithms/amplitude_estimators/ae.py 56 61 91.8%
qiskit/algorithms/amplitude_estimators/mlae.py 46 51 90.2%
qiskit/algorithms/amplitude_estimators/fae.py 39 45 86.67%
qiskit/algorithms/amplitude_estimators/iae.py 59 65 90.77%
Files with Coverage Reduction New Missed Lines %
qiskit/algorithms/amplitude_estimators/fae.py 1 94.59%
Totals Coverage Status
Change from base Build 3116195495: 0.1%
Covered Lines: 59596
Relevant Lines: 70598

💛 - Coveralls

@manoelmarques manoelmarques changed the title [WIP] Rewrite Amplitude Estimators with Primitives Rewrite Amplitude Estimators with Primitives Sep 6, 2022
@manoelmarques manoelmarques marked this pull request as ready for review September 6, 2022 02:10
Copy link
Contributor

@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.

I left some comments below, some of them apply to other flavors of QAE but I only commented on one occurrence, eg about treating the sampler as shot-based vs. statevector 🙂

qiskit/algorithms/amplitude_estimators/ae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/ae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/ae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/fae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/fae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/ae.py Outdated Show resolved Hide resolved
Co-authored-by: Julien Gacon <gaconju@gmail.com>
qiskit/algorithms/amplitude_estimators/ae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/fae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/fae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/iae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/iae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/iae.py Outdated Show resolved Hide resolved
manoelmarques and others added 4 commits September 6, 2022 13:38
Co-authored-by: dlasecki <dal@zurich.ibm.com>
Co-authored-by: dlasecki <dal@zurich.ibm.com>
Co-authored-by: dlasecki <dal@zurich.ibm.com>
Co-authored-by: dlasecki <dal@zurich.ibm.com>
qiskit/algorithms/amplitude_estimators/ae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/ae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/ae.py Outdated Show resolved Hide resolved
qiskit/algorithms/amplitude_estimators/ae_utils.py Outdated Show resolved Hide resolved
dlasecki
dlasecki previously approved these changes Sep 23, 2022
dlasecki
dlasecki previously approved these changes Sep 23, 2022
manoelmarques and others added 2 commits September 23, 2022 16:54
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

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

@manoelmarques Thx, LGTM.

@mergify mergify bot merged commit ae0ceee into Qiskit:main Sep 24, 2022
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Jun 27, 2023
* Rewrite Amplitude Estimators with Primitives

* Update qiskit/algorithms/amplitude_estimators/ae.py

Co-authored-by: Julien Gacon <gaconju@gmail.com>

* fix errors

* Update qiskit/algorithms/amplitude_estimators/ae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* Update qiskit/algorithms/amplitude_estimators/fae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* Update qiskit/algorithms/amplitude_estimators/fae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* Update qiskit/algorithms/amplitude_estimators/iae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* fix annotations

* Add sampler properties

* refactor evaluate_measurements

* deprecate qiskit.pulse.utils.deprecated_functionality in favor of qiskit.utils.deprecation.deprecate_function (Qiskit#8696)

* deprecate deprecated_functionality

* pylint: disable=cyclic-import

* Add reno

* Include Terra version in deprecation notice

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

* feat: support sum(LinearMixin) (Qiskit#8722)

* feat: support sum(LinearMixin)

This enables the use of `sum(...)` for subclasses of the `LinearMixin`
class. It also adds the reflective operand dunder methods `__radd__` and
`__rsub__`.

* Add crossreferences to release note

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

* set shots on fae sampler

* cache circuits during estimate

* Change algos estimate

* Change from run_options ro options

* remove circuits cache list

* Update qiskit/algorithms/amplitude_estimators/ae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* Update qiskit/algorithms/amplitude_estimators/ae_utils.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* change slice

* Get shots from metadata

* Update qiskit/algorithms/amplitude_estimators/iae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* Rearrange annotation None

* Update qiskit/algorithms/amplitude_estimators/ae.py

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Fix deprecation msg

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: dlasecki <dal@zurich.ibm.com>
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: Max Rossmannek <oss@zurich.ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
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
* Rewrite Amplitude Estimators with Primitives

* Update qiskit/algorithms/amplitude_estimators/ae.py

Co-authored-by: Julien Gacon <gaconju@gmail.com>

* fix errors

* Update qiskit/algorithms/amplitude_estimators/ae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* Update qiskit/algorithms/amplitude_estimators/fae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* Update qiskit/algorithms/amplitude_estimators/fae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* Update qiskit/algorithms/amplitude_estimators/iae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* fix annotations

* Add sampler properties

* refactor evaluate_measurements

* deprecate qiskit.pulse.utils.deprecated_functionality in favor of qiskit.utils.deprecation.deprecate_function (Qiskit/qiskit#8696)

* deprecate deprecated_functionality

* pylint: disable=cyclic-import

* Add reno

* Include Terra version in deprecation notice

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

* feat: support sum(LinearMixin) (Qiskit/qiskit#8722)

* feat: support sum(LinearMixin)

This enables the use of `sum(...)` for subclasses of the `LinearMixin`
class. It also adds the reflective operand dunder methods `__radd__` and
`__rsub__`.

* Add crossreferences to release note

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

* set shots on fae sampler

* cache circuits during estimate

* Change algos estimate

* Change from run_options ro options

* remove circuits cache list

* Update qiskit/algorithms/amplitude_estimators/ae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* Update qiskit/algorithms/amplitude_estimators/ae_utils.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* change slice

* Get shots from metadata

* Update qiskit/algorithms/amplitude_estimators/iae.py

Co-authored-by: dlasecki <dal@zurich.ibm.com>

* Rearrange annotation None

* Update qiskit/algorithms/amplitude_estimators/ae.py

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Fix deprecation msg

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: dlasecki <dal@zurich.ibm.com>
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: Max Rossmannek <oss@zurich.ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
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
Labels
Changelog: Deprecation Include in "Deprecated" section of changelog Changelog: New Feature Include in the "Added" section of the changelog mod: algorithms Related to the Algorithms module
Projects
None yet
8 participants