-
Notifications
You must be signed in to change notification settings - Fork 586
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
Added TritFlip #5784
Added TritFlip #5784
Conversation
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.
Hi @albi3ro, @glassnotes, @mudit2812, and @trbromley, this is the TritFlip PR, I'm excited to get your feedback!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5784 +/- ##
==========================================
- Coverage 99.66% 99.66% -0.01%
==========================================
Files 415 415
Lines 39601 39372 -229
==========================================
- Hits 39469 39239 -230
- Misses 132 133 +1 ☔ View full report in Codecov by Sentry. |
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.
Thanks @Gabriel-Bottrill ! Implementation looks fine, mostly comments about language and documentation
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.
I agree the "parameter flipping" language was, bad I updated it and hopefully it is more clear now.
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.
Thanks for making these changes, @Gabriel-Bottrill . Implementation looks good to me; maybe someone on the Xanadu side can weigh in on whether the cosmetic changes to QutritAmplitudeDamping
should be moved to a separate PR, or if it is fine to have them here.
Co-authored-by: Olivia Di Matteo <2068515+glassnotes@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.
Looks great, thanks @Gabriel-Bottrill !
**Context:** The QutritAmplitudeDamping channel #5503 and #5757 variable names are named with numbers, when making the TritFlip channel #5784 I realized that it would be better to specify subspaces for clarity. This PR is meant to change the name of QutritAmplitudeDamping's variables. **Description of the Change:** Change the name of the QutritAmplitudeDamping channel's variables from gamma_1, gamma_2, and gamma_3 to gamma_01, gamma_02, and gamma_12, respectively. **Benefits:** Makes the code more clear for which variables affect which subspace, reducing the need to check the docs. **Possible Drawbacks:** Changes the name of variables of development code. **Related GitHub Issues:** N/A --------- Co-authored-by: Gabriel Bottrill <bottrill@student.ubc.ca> Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com>
Context:
default.qutrit.mixed
device has been added, but only two channels have been added , this adds the third channel to the device so the device can simulate the qutrit equivalent of bitflip.Description of the Change:
Adds TritFlip channel which allows for "bitflips" between subspaces of the qutrit.
Benefits:
Makes it possible to simulate single state parameter flips.
Possible Drawbacks:
The parameter flips are done together, this makes it impossible to write the partial derivatives parameter shift rules generally. Also it is inefficient if you only want to simulate one flip occurring, such as just 1 and 2.
Related GitHub Issues:
N/A