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

Update VJP and adjoint-jacobian methods #222

Merged
merged 15 commits into from
Feb 8, 2022
Merged

Conversation

maliasadi
Copy link
Member

@maliasadi maliasadi commented Feb 2, 2022

Context:
This PR includes the updates for VJP and AdjDiff in a way that the AdjointJacobian method is more efficient (with reducing the cost of finding trainable params in the list of operations and updating jacobian matrix cache-efficiently) and VJP methods return processing functions required to compute the VJPs.

Description of the Change:

  • Add JacobianData to encapsulate the data required in AdjDiff and VJP methods.
  • Update the implm. of adjointJacobian and add adjointJacobianJD with a new optimization.
  • Update the C++ implm. of vectorJacobianProduct using lambda functions.
  • Update the python bindings for adjointJacobian and VJP methods in lightning.qubit.
  • Update C++ and Python tests.

Benchmark:
The new adjoint-jacobian is up to 6% faster than the current implementation for large enough circuits.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2022

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@codecov
Copy link

codecov bot commented Feb 2, 2022

Codecov Report

Merging #222 (680ee5c) into master (a01bf62) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #222   +/-   ##
=======================================
  Coverage   99.71%   99.71%           
=======================================
  Files           4        4           
  Lines         351      354    +3     
=======================================
+ Hits          350      353    +3     
  Misses          1        1           
Impacted Files Coverage Δ
pennylane_lightning/_version.py 100.00% <100.00%> (ø)
pennylane_lightning/lightning_qubit.py 99.62% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a01bf62...680ee5c. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2022

Test Report (C++) on Ubuntu

       1 files  ±  0         1 suites  ±0   0s ⏱️ ±0s
   296 tests ±  0     296 ✔️ ±  0  0 💤 ±0  0 ±0 
2 687 runs   - 62  2 687 ✔️  - 62  0 💤 ±0  0 ±0 

Results for commit 680ee5c. ± Comparison against base commit a01bf62.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@chaeyeunpark chaeyeunpark left a comment

Choose a reason for hiding this comment

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

Hi Ali, nice work 👍 I here just drop several comments.

pennylane_lightning/lightning_qubit.py Show resolved Hide resolved
pennylane_lightning/src/algorithms/AdjointDiff.hpp Outdated Show resolved Hide resolved
Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Great work on this @maliasadi
I have some questions and thoughts. I reckon we can figure out the integration testing mentioned earlier and identify how to best examine the use with PL.

Maybe @josh146 would have an idea how to best do some runtime and integration testing with/against the Python-only implementation.

pennylane_lightning/lightning_qubit.py Outdated Show resolved Hide resolved
pennylane_lightning/lightning_qubit.py Show resolved Hide resolved
pennylane_lightning/src/algorithms/AdjointDiff.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@chaeyeunpark chaeyeunpark left a comment

Choose a reason for hiding this comment

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

Hi Ali, I just wanna approve this PR. Great job 👍

Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Excellent work on this @maliasadi
Happy to approve too. Once the PL changes come we can do the integration.

@maliasadi maliasadi merged commit 64dc2e3 into master Feb 8, 2022
@maliasadi maliasadi deleted the up_aj_vjp_methods branch February 8, 2022 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants