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

Fixes a bug where decompositions would reset the differentiation method of a QNode #1117

Merged
merged 4 commits into from
Mar 2, 2021

Conversation

josh146
Copy link
Member

@josh146 josh146 commented Feb 28, 2021

Context: During tape construction, the differentiation options of the tape are set prior to any potential tape expansion. Since tape expansion is a tape-to-tape transformation that outputs a new tape, if a decomposition is required, the tape differentiation options are lost.

Description of the Change: Moves the setting of the differentiation options to after potential tape expansion.

Benefits: Preserves differentiation options, even if a decomposition is performed.

Possible Drawbacks: We plan to move tape expansion to the device, as a result, the logic that causes this bug within the QNode will be removed. This is not a drawback, just something to keep in mind --- an expected future change would have also eliminated this bug!

Related GitHub Issues: Closes #1115

@josh146 josh146 added the bug 🐛 Something isn't working label Feb 28, 2021
@codecov
Copy link

codecov bot commented Feb 28, 2021

Codecov Report

Merging #1117 (a971beb) into master (8a3eab8) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1117   +/-   ##
=======================================
  Coverage   97.74%   97.74%           
=======================================
  Files         154      154           
  Lines       11728    11728           
=======================================
  Hits        11463    11463           
  Misses        265      265           
Impacted Files Coverage Δ
pennylane/tape/qnode.py 98.93% <100.00%> (ø)

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 8a3eab8...a971beb. Read the comment docs.

Copy link
Contributor

@antalszava antalszava left a comment

Choose a reason for hiding this comment

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

Looks good to me! 💯 Thank you so much for catching & fixing this! 😊

josh146 and others added 2 commits March 2, 2021 20:16
@josh146 josh146 merged commit bf36183 into master Mar 2, 2021
@josh146 josh146 deleted the fixes-1115 branch March 2, 2021 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Device jacobian is not called when QNode contains gates that need to be decomposed
2 participants