Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

Fix probability Hessians with the parameter shift method. #1367

Merged
merged 209 commits into from
Nov 28, 2020

Conversation

Zoufalc
Copy link
Contributor

@Zoufalc Zoufalc commented Oct 18, 2020

Summary

This is a minor fix for the gradient framework. This enables the evaluation of probability Hessians with the parameter shift rule.

Details and comments

The respective combo function was adapted such that it is compatible with values that have been computed using a parameter shift probability gradient .

Cryoris and others added 30 commits September 22, 2020 15:07
Clean PR with the current gradient state
…nother look. Same goes for some of the class method names inside of LinCombQFI and OverlapQFI
* Update derivatives_base.py

Added some doc-strings to several methods inside of derivatives_base.py. Also simplified the logic of `erase_operator_coeffs` by removing some leftover logic from previous implementations.

* Update qiskit/aqua/operators/gradients/derivatives_base.py

Co-authored-by: Julien Gacon <jul@zurich.ibm.com>
This notebook will be removed after today - sorry but this is the best way to share this notebook with @bryce
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'm a bit confused by this PR: in the typehints it says that the input (x) always must be a state function -- why do we explicitly need to check this? If something is passed in that is not a state function I think the error lies somewhere earlier in this code, since we should be able to assume that type hints are always obeyed

@@ -261,9 +266,16 @@ def get_primitives(item):
item = item.primitive.data
return item

is_statefn = False
if isinstance(x, Iterable):
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the typehint above, this is the case of x being a list? I think it would be a little bit clearer if we could change this to be the same as above 😄

if isinstance(x, list):

Copy link
Contributor Author

@Zoufalc Zoufalc Oct 25, 2020

Choose a reason for hiding this comment

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

I suppose we could drop the 'StateFn' check. In my point of view the type hint supports the user to get more insight on what is going on.

@woodsp-ibm woodsp-ibm requested a review from Cryoris October 27, 2020 15:57
@manoelmarques manoelmarques merged commit b77566c into qiskit-community:master Nov 28, 2020
manoelmarques added a commit to manoelmarques/qiskit-terra that referenced this pull request Dec 2, 2020
…munity/qiskit-aqua#1367)

* clean PR with the current gradient state

* Update derivatives_base.py

* Update hessian.py

* Update test_grad.py

* add notebooks

* Typing, Linting

* Update gradient_framework.ipynb

* typing qfi

* enable gradients for VQAlgorithms

* remove old files and notes

* enfoce num chars <= 100

* fix an import

* re-adjust import

* fix other imports

* additional base classes

* test cases pass and the core logic all works. Doc strings could use another look. Same goes for some of the class method names inside of LinCombQFI and OverlapQFI

* Update derivatives_base.py (qiskit-community/qiskit-aqua#75)

* Update derivatives_base.py

Added some doc-strings to several methods inside of derivatives_base.py. Also simplified the logic of `erase_operator_coeffs` by removing some leftover logic from previous implementations.

* Update qiskit/aqua/operators/gradients/derivatives_base.py

Co-authored-by: Julien Gacon <jul@zurich.ibm.com>

* update ordering/naming

* reshuffle, rename

* add VQE gradient unittest

* add notebook B2Meeting

This notebook will be removed after today - sorry but this is the best way to share this notebook with @bryce

* Update gradient_framework_reduced.ipynb

* removed a leftover notebook file

* removed the notebooks (and am pushing them to a cleaned up QGradients repo)

* Update gradient.py

This small change prevents trivial/zero operators from appearing in SummedOps during product rule computations.

* will be deleted need this to pull sry

* fix qfi docstring

* Add retworkx version check

* delete notebooks, fixed seed fin_diff circSampler, fix kwargs import error

* Delete test_natural_gradient.py

* fix symbol map in parameter expression grad

* typing

* Update qfi.py

* typing

* Update test_grad.py

* added a docstring to unroll_operator

* Update hessian.py

* Update gradient.py

* Fix statevector check (not always a QuantumInstance)

* Update hessian.py

* fix lint of qiskit/aqua/operators/gradients/*.py

* fix lint of qiskit/aqua/operators/gradients/circuit_gradients/*.py

* fix lint of qiskit/aqua/operators/gradients/circuit_qfis/*.py

* fix test_grad

* Copy folder

* remove unknown aqua changes

* fix ListOp, VQAlgorithms for gradients

* VQE Gradients import

* update operators init

* rm some changes not related to gradients

* mypy fixes

* skip jax test if not installed

* try to fix cyclic import

* fix lint in isinstance

* uncomment vqe tests + consistent HAS_JAX

* mypy fixes

* ignore mypy warning from opbase.coeff

* fix isinstance used w/ typehints

* try fixing sphinx

* ignore cases mypy doesn't understand correctly

* fix unnecessary else after raise

* Add jax to github actions

* fix spell

* add jax dependency to actions lint

* fix spell

* Reduce VQE Iterations in unittest

* Use ImportError

* fix lint

* Add gradient framework release note

* update gradient framework release note

* fix reno formatting errors

* Update to documentation

* Fix doctring to avoid warnings

* update gradient init

* update qaoa docstring

* remover retworkx check

* update docstrings

* lint fixes

* update docstrings to hint max_evals_grouped deprecation if gradient is given

* Update test_grad.py

Several high-level function calls still used a `method=...` argument in the initialization; however, we changed this to `grad_method`, `hess_method`, `qfi_method`. 
So I updated these arguments. 

This was causing some tests to not be carried out as the default argument was inferred when the correct argument name was not assigned a method type.

* Update test_grad.py

Added a test case for Hessian.py that forces it to differentiate an operator with a custom combo_fn

* Update lin_comb circuit gradients to fix hessian evaluation

* Move renamed gradient test into operator file

* update lin_comb

* update lin_comb gradients

* fix lint

* fix lin_comb

* fixed the bugs that were causing Hessian logic to fail with custom combo_fns

* increase coverage of natural gradient

* merge zoufalc gradients

* try all combinations of grad_method and qfi_method

* style fix

* updates for hessians

* remove redundant line

* make fixes

* remove hessian from grad

* relax VQE test

* remove redundant lambda

* fix identation

* fix spelling

* fix whitespace

* revert qfi_method in test_natural_gradient

* revert param name

* fix lint

* remove redundant comment

* update identation

* fix np.random.seed

* increase coverage for lin_comb

* fix lint

* lint

* whitespace fix

* docstring

* lint

* disable misspelling

* add yy, zz to pylintdict

* Update __init__.py

updated the gradients  __init__ file with some of the content from the release note

* fixed linting

* update init file

* fix spell

* fix spelling

* remove redundant import

* attempt to fix sphinx qiskit-community/qiskit-aqua#1

* Update qiskit/aqua/operators/gradients/qfi.py

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

* try to fix sphinx no.2

* add words to pylintdict

* fix cryoris comments

* Update qiskit/aqua/operators/gradients/circuit_qfis/lin_comb_full.py

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

* Update qiskit/aqua/operators/gradients/circuit_qfis/lin_comb_full.py

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

* Update qiskit/aqua/operators/gradients/circuit_qfis/lin_comb_full.py

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

* cryoris comments

* delete unneccessary

* Update qiskit/aqua/operators/gradients/hessian.py

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

* raise warning hessians

* include docstring TypeError

* Update qiskit/aqua/operators/gradients/circuit_qfis/overlap_diag.py

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

* Update qiskit/aqua/operators/gradients/gradient.py

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

* remove unnecessary os comment

* Update qiskit/aqua/operators/gradients/circuit_gradients/lin_comb.py

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

* Insert proper Error

* apply changes from code review

* Fix probability hessians parameter shift

* change type hint

* update docstrings

* fix typos

Co-authored-by: Cryoris <jules.gacon@googlemail.com>
Co-authored-by: Takashi Imamichi <imamichi@jp.ibm.com>
Co-authored-by: Bryce-Fuller <Bryce.Fuller@ibm.com>
Co-authored-by: Julien Gacon <jul@zurich.ibm.com>
Co-authored-by: Manoel Marques <manoel.marques@ibm.com>
Co-authored-by: Manoel Marques <manoel@us.ibm.com>
Co-authored-by: woodsp <woodsp@us.ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
manoelmarques added a commit to manoelmarques/qiskit-terra that referenced this pull request Dec 7, 2020
…munity/qiskit-aqua#1367)

* clean PR with the current gradient state

* Update derivatives_base.py

* Update hessian.py

* Update test_grad.py

* add notebooks

* Typing, Linting

* Update gradient_framework.ipynb

* typing qfi

* enable gradients for VQAlgorithms

* remove old files and notes

* enfoce num chars <= 100

* fix an import

* re-adjust import

* fix other imports

* additional base classes

* test cases pass and the core logic all works. Doc strings could use another look. Same goes for some of the class method names inside of LinCombQFI and OverlapQFI

* Update derivatives_base.py (qiskit-community/qiskit-aqua#75)

* Update derivatives_base.py

Added some doc-strings to several methods inside of derivatives_base.py. Also simplified the logic of `erase_operator_coeffs` by removing some leftover logic from previous implementations.

* Update qiskit/aqua/operators/gradients/derivatives_base.py

Co-authored-by: Julien Gacon <jul@zurich.ibm.com>

* update ordering/naming

* reshuffle, rename

* add VQE gradient unittest

* add notebook B2Meeting

This notebook will be removed after today - sorry but this is the best way to share this notebook with @bryce

* Update gradient_framework_reduced.ipynb

* removed a leftover notebook file

* removed the notebooks (and am pushing them to a cleaned up QGradients repo)

* Update gradient.py

This small change prevents trivial/zero operators from appearing in SummedOps during product rule computations.

* will be deleted need this to pull sry

* fix qfi docstring

* Add retworkx version check

* delete notebooks, fixed seed fin_diff circSampler, fix kwargs import error

* Delete test_natural_gradient.py

* fix symbol map in parameter expression grad

* typing

* Update qfi.py

* typing

* Update test_grad.py

* added a docstring to unroll_operator

* Update hessian.py

* Update gradient.py

* Fix statevector check (not always a QuantumInstance)

* Update hessian.py

* fix lint of qiskit/aqua/operators/gradients/*.py

* fix lint of qiskit/aqua/operators/gradients/circuit_gradients/*.py

* fix lint of qiskit/aqua/operators/gradients/circuit_qfis/*.py

* fix test_grad

* Copy folder

* remove unknown aqua changes

* fix ListOp, VQAlgorithms for gradients

* VQE Gradients import

* update operators init

* rm some changes not related to gradients

* mypy fixes

* skip jax test if not installed

* try to fix cyclic import

* fix lint in isinstance

* uncomment vqe tests + consistent HAS_JAX

* mypy fixes

* ignore mypy warning from opbase.coeff

* fix isinstance used w/ typehints

* try fixing sphinx

* ignore cases mypy doesn't understand correctly

* fix unnecessary else after raise

* Add jax to github actions

* fix spell

* add jax dependency to actions lint

* fix spell

* Reduce VQE Iterations in unittest

* Use ImportError

* fix lint

* Add gradient framework release note

* update gradient framework release note

* fix reno formatting errors

* Update to documentation

* Fix doctring to avoid warnings

* update gradient init

* update qaoa docstring

* remover retworkx check

* update docstrings

* lint fixes

* update docstrings to hint max_evals_grouped deprecation if gradient is given

* Update test_grad.py

Several high-level function calls still used a `method=...` argument in the initialization; however, we changed this to `grad_method`, `hess_method`, `qfi_method`. 
So I updated these arguments. 

This was causing some tests to not be carried out as the default argument was inferred when the correct argument name was not assigned a method type.

* Update test_grad.py

Added a test case for Hessian.py that forces it to differentiate an operator with a custom combo_fn

* Update lin_comb circuit gradients to fix hessian evaluation

* Move renamed gradient test into operator file

* update lin_comb

* update lin_comb gradients

* fix lint

* fix lin_comb

* fixed the bugs that were causing Hessian logic to fail with custom combo_fns

* increase coverage of natural gradient

* merge zoufalc gradients

* try all combinations of grad_method and qfi_method

* style fix

* updates for hessians

* remove redundant line

* make fixes

* remove hessian from grad

* relax VQE test

* remove redundant lambda

* fix identation

* fix spelling

* fix whitespace

* revert qfi_method in test_natural_gradient

* revert param name

* fix lint

* remove redundant comment

* update identation

* fix np.random.seed

* increase coverage for lin_comb

* fix lint

* lint

* whitespace fix

* docstring

* lint

* disable misspelling

* add yy, zz to pylintdict

* Update __init__.py

updated the gradients  __init__ file with some of the content from the release note

* fixed linting

* update init file

* fix spell

* fix spelling

* remove redundant import

* attempt to fix sphinx qiskit-community/qiskit-aqua#1

* Update qiskit/aqua/operators/gradients/qfi.py

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

* try to fix sphinx no.2

* add words to pylintdict

* fix cryoris comments

* Update qiskit/aqua/operators/gradients/circuit_qfis/lin_comb_full.py

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

* Update qiskit/aqua/operators/gradients/circuit_qfis/lin_comb_full.py

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

* Update qiskit/aqua/operators/gradients/circuit_qfis/lin_comb_full.py

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

* cryoris comments

* delete unneccessary

* Update qiskit/aqua/operators/gradients/hessian.py

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

* raise warning hessians

* include docstring TypeError

* Update qiskit/aqua/operators/gradients/circuit_qfis/overlap_diag.py

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

* Update qiskit/aqua/operators/gradients/gradient.py

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

* remove unnecessary os comment

* Update qiskit/aqua/operators/gradients/circuit_gradients/lin_comb.py

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

* Insert proper Error

* apply changes from code review

* Fix probability hessians parameter shift

* change type hint

* update docstrings

* fix typos

Co-authored-by: Cryoris <jules.gacon@googlemail.com>
Co-authored-by: Takashi Imamichi <imamichi@jp.ibm.com>
Co-authored-by: Bryce-Fuller <Bryce.Fuller@ibm.com>
Co-authored-by: Julien Gacon <jul@zurich.ibm.com>
Co-authored-by: Manoel Marques <manoel.marques@ibm.com>
Co-authored-by: Manoel Marques <manoel@us.ibm.com>
Co-authored-by: woodsp <woodsp@us.ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants