-
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
Fix derivatives of merge_rotations
and single_qubit_fusion
#6033
Conversation
…and test for singular points of Jacobian
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 #6033 +/- ##
==========================================
- Coverage 99.66% 99.65% -0.01%
==========================================
Files 432 432
Lines 41996 41691 -305
==========================================
- Hits 41855 41549 -306
- Misses 141 142 +1 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Vincent Michaud-Rioux <vincent.michaud-rioux@xanadu.ai>
Co-authored-by: Korbinian Kottmann <43949391+Qottmann@users.noreply.github.com>
Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com>
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.
Nice PR @dwierichs . Looks good to me! I'm suggesting a couple changes, which you can accept or reject as you see fit.
Haha, sorry @Qottmann . Yes the base branch is right, but I merged master into both branches, but only pushed it here and not on the |
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.
Good job @dwierichs
Don't see anything suspicious and appreciate the simplifications of the code.
Context:
As reported in #5715,
merge_rotations
andsingle_qubit_fusion
have problems with differentiability at specific points.#6031 takes care of upgrading the Rot-angle fusion to only ever return NaNs at mathematically non-differentiable points, rather than wrong results.
However, the transforms add additional points, based on internal optimizations, where the derivative is flawed.
Description of the Change:
This PR fixes the flawed derivatives caused by the code of the transforms themselves.
Benefits:
Fixes derivatives of
merge_rotations
andsingle_qubit_fusion
(where mathematically defined)Possible Drawbacks:
N/A
Related GitHub Issues:
Fixes another part of #5715, still not all of it.