-
Notifications
You must be signed in to change notification settings - Fork 603
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
Add generator checks and differentiability checks to assert_valid #6282
Conversation
Hello. You may have forgotten to update the changelog!
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6282 +/- ##
=======================================
Coverage 99.39% 99.40%
=======================================
Files 446 446
Lines 42328 42358 +30
=======================================
+ Hits 42073 42104 +31
+ Misses 255 254 -1 ☔ View full report in Codecov by Sentry. |
…o assert-valid
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.
Other than a changelog entry , everything looks good to me 👍
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.
This looks great to me. Just a couple of questions
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!
) The purpose of this story is to check if every operator can be differentiated properly with the parameter shift method. The gradient calculated using parameter shift is compared with backprop to verify that they are equal. There are some cases where this does not apply: 1. Backprop does not produce the correct gradient in some cases such as for `StatePrep` and for `QubitUnitary`, as it does not take into account the constraint that the matrix must remain unitary, making this test invalid. 2. Some operator takes integers as parameters, such as `BasisState`. In this case, it does not make sense to take the gradient of integer parameters. For these cases, a `skip_differentiation` toggle is added to `assert_valid` such that the differentiation check is skipped for these operators. Three bugs are found as a result of adding this check. The relevant tests are xfailed: #6331 #6333 #6340 Some other minor bug fixes are also included in this PR. [sc-65197] Fixes #6311
The purpose of this story is to check if every operator can be differentiated properly with the parameter shift method. The gradient calculated using parameter shift is compared with backprop to verify that they are equal.
There are some cases where this does not apply:
StatePrep
and forQubitUnitary
, as it does not take into account the constraint that the matrix must remain unitary, making this test invalid.BasisState
. In this case, it does not make sense to take the gradient of integer parameters.For these cases, a
skip_differentiation
toggle is added toassert_valid
such that the differentiation check is skipped for these operators.Three bugs are found as a result of adding this check. The relevant tests are xfailed:
#6331
#6333
#6340
Some other minor bug fixes are also included in this PR.
[sc-65197]
Fixes #6311