-
Notifications
You must be signed in to change notification settings - Fork 575
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
[Bugfix 1999] Correct attributes #2113
Conversation
Nice work! |
@mariaschuld I may hold off on reviewing until the tests are passing if that is okay! |
Sorry @josh146 I should not have tagged you before checking the tests, this 20 min wait is too much for my impatience :) |
… into correct-attributes
Codecov Report
@@ Coverage Diff @@
## master #2113 +/- ##
=======================================
Coverage 99.19% 99.19%
=======================================
Files 228 228
Lines 17534 17558 +24
=======================================
+ Hits 17393 17417 +24
Misses 141 141
Continue to review full report at Codecov.
|
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.
Looks good to me, thanks @mariaschuld!
Context:
It was noted that the existing attributes miss some gates and, in one case, contain a wrong gate.
Adding the
IsingZZ
tois_diagonal_in_z_basis
uncovered an issue: Since it does not have_eigvals
defined, the eigenvalues are computed from the matrix using a numpy module, which means they are not differentiable in a backpropagation pipeline.I think this is an issue with inferring the eigenvalues from a matrix in a non-diffable manner, which goes against the grain of PL. For now we fix it by warning of this issue.
Description of the Change:
Remove/add gates. Add
_eigvals
method toIsingZZ
and explain the_eigvals
requirement in the attributes docstring.Benefits: Fixes #1999