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

Fidelity interface using primitives #8303

Merged
merged 106 commits into from
Sep 2, 2022
Merged

Conversation

ElePT
Copy link
Contributor

@ElePT ElePT commented Jul 6, 2022

Summary

We propose the addition of a series of primitive-based Fidelity interfaces that will become a fundamental building block for primitive-based algorithms: VQD, PVQD, QNSPSA optimizers and quantum kernels.

This PR contains a proposal for BaseStateFidelity and the ComputerUncompute method interface that is used in the refactored quantum kernel class (currently WIP, see design doc and PR). Other methods for calculating state overlaps/ fidelities could be added in the future (i.e. Swap tests, Hadamard tests...).

Details and comments

This PR closes #6876 and #8493.

Update (August 31)

The current implementation follows the design of the new primitive interface, with a run method that returns an AlgorithmJob that is non-blocking. The AlgorithmJob class has been introduced for typing purposes, but in the future it might be modified to include additional/different functionality to PrimitiveJob.

@ElePT ElePT requested review from a team, ikkoham and t-imamichi as code owners July 6, 2022 10:33
@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

Pull Request Test Coverage Report for Build 2981696422

  • 126 of 134 (94.03%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 84.224%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/algorithms/state_fidelities/base_state_fidelity.py 73 77 94.81%
qiskit/algorithms/state_fidelities/compute_uncompute.py 31 35 88.57%
Totals Coverage Status
Change from base Build 2979080470: 0.03%
Covered Lines: 57176
Relevant Lines: 67886

💛 - Coveralls

qiskit/primitives/fidelity/base_fidelity.py Outdated Show resolved Hide resolved
qiskit/primitives/fidelity/fidelity.py Outdated Show resolved Hide resolved
qiskit/primitives/fidelity/fidelity.py Outdated Show resolved Hide resolved
test/python/primitives/fidelity/test_fidelity.py Outdated Show resolved Hide resolved
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.

Overall, it looks reasonable.

qiskit/primitives/fidelity/base_fidelity.py Outdated Show resolved Hide resolved
qiskit/primitives/fidelity/fidelity.py Outdated Show resolved Hide resolved
qiskit/primitives/fidelity/fidelity.py Outdated Show resolved Hide resolved
qiskit/primitives/fidelity/fidelity.py Outdated Show resolved Hide resolved
qiskit/primitives/fidelity/fidelity.py Outdated Show resolved Hide resolved
qiskit/primitives/fidelity/fidelity.py Outdated Show resolved Hide resolved
test/python/primitives/fidelity/test_fidelity.py Outdated Show resolved Hide resolved
test/python/primitives/fidelity/test_fidelity.py Outdated Show resolved Hide resolved
test/python/primitives/fidelity/test_fidelity.py Outdated Show resolved Hide resolved
gentinettagian and others added 2 commits July 16, 2022 11:09
@ElePT ElePT requested a review from eggerdj as a code owner July 18, 2022 15:53
Cryoris
Cryoris previously approved these changes Sep 2, 2022
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.

LGTM! Let's see if the docs build passes.... I'll also hold off automerge in case @woodsp-ibm or anybody else wants to have another look!

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
ElePT and others added 5 commits September 2, 2022 20:59
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
…ecaa8dd.yaml

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
@ElePT
Copy link
Contributor Author

ElePT commented Sep 2, 2022

Thank you for checking the docs, @woodsp-ibm! I applied all of your suggestions.

fidelities=fidelities,
raw_fidelities=raw_fidelities,
metadata=result.metadata,
run_options=run_opts,
Copy link
Member

Choose a reason for hiding this comment

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

This is just a comment. In gradients, in the result run_options there it says of run_options

run_options for the estimator. Currently, estimator's default run_options is not included.

which is the case here right. So these end up being the options we give to sampler.run() which may then augment them with it own default option.s If run_options were part of Sampler ( and Estimator) result then like metadata it could be result.run_options. But until such time that this happens, its just perhaps something to bear in mind.

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
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.

Thanks a lot for all the effort and patience @ElePT 😁

@woodsp-ibm
Copy link
Member

Thanks a lot for all the effort and patience @ElePT

Seconded!

@mergify mergify bot merged commit 9ddc97d into Qiskit:main Sep 2, 2022
ElePT added a commit to ElePT/qiskit that referenced this pull request Jun 27, 2023
* fidelity_poc

* init stuff

* simplified design

* unittests

* unittest working

* optional parameters

* typo

* documentation

* Update test/python/primitives/fidelity/test_fidelity.py

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

* Update qiskit/primitives/fidelity/base_fidelity.py

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

* black

* import order in test fixed

* docstring fixed

* Update qiskit/primitives/fidelity/base_fidelity.py

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

* context removed

* Reset branch

* Add init

* Fix inits

* lint fixes

* removed abstractmethod label for set_circuits

* added support for setting only one circuit

* new unittest

* Move fidelities to algorithms

* Update interface with new design doc

* Fix docstrings and unit tests

* Add evaluate

* Adapt signature run

* Make job async

* Fix types

* Remove async, cache circuits

* Update evaluate method, replace use of np

* Remove circuits from init

* Add async, tests, update docstrings

* Add default run options

* change names, abstract methods, improve docstrings

* Update docstrings, typing

* Fix style

* Update qiskit/algorithms/fidelities/base_fidelity.py

Co-authored-by: Declan Millar <declan.millar@ibm.com>

* Update qiskit/algorithms/fidelities/base_fidelity.py

Co-authored-by: Declan Millar <declan.millar@ibm.com>

* Make async

* Add FidelityResult

* Update qiskit/algorithms/fidelities/__init__.py

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

* Add to algorithms init

* Rename classes

* Apply review comments

* Update preprocessing

* Update docstrings

* Move run to base class

* Remove left/right params

* Apply reviews steve/julien/hamamura

* Fix typo

* Style changes

* Allow different num.params in circuit 1 and 2

* Fix unittest typo

* Fix black

* Fix docs

* Fix style/docstrings

* Add AlgorithmJob

* Add truncate, fix result docstring

* Completely remove left-right

* Add comments

* Make create circuit public

* Remove empty param distinction

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

* change list preprocessing

* Update qiskit/algorithms/algorithm_job.py

Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Apply comments Steve

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update test/python/algorithms/state_fidelities/test_compute_uncompute.py

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

* Style changes

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Apply comments Julien

* Style fix

* Fix bug for lists of numpy values

* Support single circuits

* Support single circuits

* Add fidelity interface using primitives

* Apply reviews

* Fix docs

* Fix docs

* Fix docs

* Fix lint

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update qiskit/algorithms/state_fidelities/compute_uncompute.py

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

* Update releasenotes/notes/add-fidelity-interface-primitives-dc543d079ecaa8dd.yaml

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

* Update qiskit/algorithms/state_fidelities/__init__.py

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

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

Co-authored-by: Gian Gentinetta <gian.gentinetta@gmx.ch>
Co-authored-by: Gian Gentinetta <31244916+gentinettagian@users.noreply.github.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: dlasecki <dal@zurich.ibm.com>
Co-authored-by: Declan Millar <declan.millar@ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>
Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
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
* fidelity_poc

* init stuff

* simplified design

* unittests

* unittest working

* optional parameters

* typo

* documentation

* Update test/python/primitives/fidelity/test_fidelity.py

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

* Update qiskit/primitives/fidelity/base_fidelity.py

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

* black

* import order in test fixed

* docstring fixed

* Update qiskit/primitives/fidelity/base_fidelity.py

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

* context removed

* Reset branch

* Add init

* Fix inits

* lint fixes

* removed abstractmethod label for set_circuits

* added support for setting only one circuit

* new unittest

* Move fidelities to algorithms

* Update interface with new design doc

* Fix docstrings and unit tests

* Add evaluate

* Adapt signature run

* Make job async

* Fix types

* Remove async, cache circuits

* Update evaluate method, replace use of np

* Remove circuits from init

* Add async, tests, update docstrings

* Add default run options

* change names, abstract methods, improve docstrings

* Update docstrings, typing

* Fix style

* Update qiskit/algorithms/fidelities/base_fidelity.py

Co-authored-by: Declan Millar <declan.millar@ibm.com>

* Update qiskit/algorithms/fidelities/base_fidelity.py

Co-authored-by: Declan Millar <declan.millar@ibm.com>

* Make async

* Add FidelityResult

* Update qiskit/algorithms/fidelities/__init__.py

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

* Add to algorithms init

* Rename classes

* Apply review comments

* Update preprocessing

* Update docstrings

* Move run to base class

* Remove left/right params

* Apply reviews steve/julien/hamamura

* Fix typo

* Style changes

* Allow different num.params in circuit 1 and 2

* Fix unittest typo

* Fix black

* Fix docs

* Fix style/docstrings

* Add AlgorithmJob

* Add truncate, fix result docstring

* Completely remove left-right

* Add comments

* Make create circuit public

* Remove empty param distinction

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

* change list preprocessing

* Update qiskit/algorithms/algorithm_job.py

Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Apply comments Steve

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update test/python/algorithms/state_fidelities/test_compute_uncompute.py

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

* Style changes

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Apply comments Julien

* Style fix

* Fix bug for lists of numpy values

* Support single circuits

* Support single circuits

* Add fidelity interface using primitives

* Apply reviews

* Fix docs

* Fix docs

* Fix docs

* Fix lint

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update qiskit/algorithms/state_fidelities/compute_uncompute.py

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

* Update releasenotes/notes/add-fidelity-interface-primitives-dc543d079ecaa8dd.yaml

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

* Update qiskit/algorithms/state_fidelities/__init__.py

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

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

* Update qiskit/algorithms/state_fidelities/base_state_fidelity.py

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

Co-authored-by: Gian Gentinetta <gian.gentinetta@gmx.ch>
Co-authored-by: Gian Gentinetta <31244916+gentinettagian@users.noreply.github.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: dlasecki <dal@zurich.ibm.com>
Co-authored-by: Declan Millar <declan.millar@ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>
Co-authored-by: Ikko Hamamura <ikkoham@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: New Feature Include in the "Added" section of the changelog mod: algorithms Related to the Algorithms module mod: primitives Related to the Primitives module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

State overlap interface
10 participants