This repository has been archived by the owner on Dec 7, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 376
Conversation
This file contains 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
Clean PR with the current gradient state
Remove old files and notes
Enforce number of character <= 100
…nother look. Same goes for some of the class method names inside of LinCombQFI and OverlapQFI
…base_classes additional base classes
* 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
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Cryoris
reviewed
Oct 15, 2020
Cryoris
reviewed
Oct 15, 2020
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Cryoris
reviewed
Oct 15, 2020
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Cryoris
approved these changes
Oct 15, 2020
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, items for backlog:
- in-code comments to make it easier for developers to understand what's going on
- try removing some deepcopies in favor of perfomance
- docstrings should contain examples and be checked for duplications (a general second iteration would be good)
- check if some staticmethods should be decoupled from the classes and put into functions so they are better reusable (what some might call a
utils
file)
woodsp-ibm
approved these changes
Oct 15, 2020
mtreinish
pushed a commit
to mtreinish/qiskit-core
that referenced
this pull request
Nov 20, 2020
* 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 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 2, 2020
* 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 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
* 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 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
Changelog: API Change
Include in the Changed section of the changelog
Changelog: New Feature
Include in the Added section of the changelog
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.
Summary
This PR adds a gradient framework implementation into the operator flow.
Details and comments
Link to Design Doc