-
Notifications
You must be signed in to change notification settings - Fork 60
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 Clifford.to_circuit
and Clifford.copy
methods + initialization via circuits
#1139
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1139 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 67 68 +1
Lines 9588 9764 +176
==========================================
+ Hits 9588 9764 +176
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Clifford.copy
method and initialization via circuitsClifford.to_circuit
and Clifford.copy
methods + initialization via circuits
@BrunoLiegiBastonLiegi @AlejandroSopena this should be ready for review (with some urgency) after passing tests |
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 @renatomello I just finished giving this a first look and it's looking good. I have got just a couple of general comments.
The first one regards the ability to pass a circuit directly to the Clifford
object constructor. From my perspective this is somewhat redundant as you can already do that with Clifford.from_circuit()
and it's not really adding any additional functionality.
Secondly I wonder whether many of the clifford_utils
functions should be moved to the CliffordBackend
object. They are extensively making use of the backend through the Clifford
object already, at this point they should probably just be part of it and called at need inside of the Clifford
.
Finally, why do you need to store the circuit inside of the Clifford
object? In general I would like to keep it as light as possible, keeping in mind that at some point we might want to create an abstract Result
object from which both this Clifford
, the QuantumState
, MeasurementOutcomes
, CircuitResult
and all the possible future outcomes object inherit from.
Redundancy is not necessarily a bad thing. For instance, we have
I have no objection to this. However, this sounds like a different PR.
You are right that it can be memory intensive to store 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.
Thanks @renatomello, everything looks good. I agree with @BrunoLiegiBastonLiegi 's comment to move some clifford_utils
functions to the CliffordBackend
.
Co-authored-by: Alejandro Sopena <44305203+AlejandroSopena@users.noreply.github.com>
Checklist: