This repository was archived by the owner on Dec 7, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 372
Improve Amplitude Estimation Algos and Tests #788
Merged
Merged
Changes from all commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
a281f69
add sine test and circuit test
Cryoris cc1734b
add docstring
Cryoris 222b851
add docstring
Cryoris cb460ec
add setter for i_objective, tests for setting of A/Q/i_obj
Cryoris 0f32b75
add test for updating A operator, test for IQAE circuit
Cryoris af9e1a7
fix safe_max/min
Cryoris d4b3182
only apply Q if the power is not 0
Cryoris 04eaa5a
only apply Q if the power is not 0
Cryoris 62c6d70
evaluation schedule must include Q^0
Cryoris ba71eb3
add test for MLAE circuits
Cryoris 08e7974
CI is based on 'mle' not 'estimation'
Cryoris e36a20c
correct definition of bubbles / add comments
Cryoris 8ad95d0
remove unnecessary definition, y should be int
Cryoris 8256230
consistent order: A op, Q op, i_obj
Cryoris 49da38f
fix typo
Cryoris 7578226
fix bug: MLE not in CI
Cryoris 8f4ecf0
update default num. of evaluations
Cryoris ec0accf
update comments
Cryoris d79f606
add docstrings, rename ci -> confint throughout
Cryoris 8530532
add "ae" to good-names, well-known, precise variable name for Amplitu…
Cryoris 6a987f4
remove too complex cases, add docstrings
Cryoris eef5a2a
fix expected value after MLAE fix
Cryoris a3277e6
add confidence interval tests
Cryoris 8ababcb
fix spell/lint/style
Cryoris bdfb98d
fix spell/lint/style
Cryoris 067f18f
fix test
Cryoris 3e1dc75
resolve merge conflict
Cryoris d143dd5
resolve merge conflicts from type hints
Cryoris 36e17ac
rename log_max_evals to m
Cryoris 03900bc
add num_oracle_queries for all AE variants
Cryoris 4b84299
Merge branch 'master' of https://github.com/Qiskit/qiskit-aqua into a…
Cryoris 66b77ea
remove QAES -- not part of this PR
Cryoris b53f231
fix lints, docstrings begin first line
Cryoris 871d908
fix lint
manoelmarques fc5ec8f
fix docstring
manoelmarques 3ff5958
fix docstring
manoelmarques 497fc9d
add type hints, fix args order in ae.py
Cryoris 86f977b
avoid using `ae` (use `qae` instead)
Cryoris 34420c7
rename `m` -> `num_oracle_circuit`
Cryoris 9688c65
fix spell & style
Cryoris 2579ba2
test circuit unitaries instead of gate counts
Cryoris b096b1a
Merge branch 'ae_tests' of github.com:Cryoris/qiskit-aqua into ae_tests
Cryoris 5618cb1
fix lint
Cryoris 9737cd2
Merge branch 'master' into ae_tests
manoelmarques ddd1db6
Merge branch 'master' into ae_tests
manoelmarques 3fd0891
update docstring
Cryoris 74f9c25
Merge branch 'ae_tests' of github.com:Cryoris/qiskit-aqua into ae_tests
Cryoris File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Can we rename this to be QPEAmplitudeEstimation, and say in the comment on line 38 that this is "The original, Quantum Phase Estimation-based Amplitude Estimation Algorithm."? I think it may be strange to have one algorithm called AmplitudeEstimation but then a base class called AmplitudeEstimationAlgorithm. It would be better to be descriptive. If so, we should also rename the file qpe_ae.py, yes?
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.
The alternative might be to rename the base class, which would be less disruptive - otherwise notebooks, test cases and any users code would need to alter too. I do not know how much, or if at all, externally that base class type might be used at present outside of the package. I suspect it could be renamed without the same impact as the algorithm derived from it.
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.
AmplitudeEstimationis used quite a bit in notebooks (and our codes), I also prefer to keep it. As discussed on Slack I'll update the the docstring though.