-
Notifications
You must be signed in to change notification settings - Fork 50
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
Some group theory tools for Quantum error correction #293
Some group theory tools for Quantum error correction #293
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.
Looks like a great start! Let's do some final polish so that we have what is currently available merged, before we worry about more sophisticated functionality. Doing small merges often is much easier for me than doing large pull requests.
Converting to draft for now. Convert back to non-draft and re-request a review when ready.
@KDGoodenough , could you check whether the tests seem reasonable for the functionality you have in mind?
src/grouptableaux.jl
Outdated
s, _, r = canonicalize!(copy(s), ranks=true) | ||
if r == 0 | ||
gs = zero(Stabilizer, 1, nqubits(s)) | ||
if (1im * zero(Stabilizer, 1, nqubits(s))[1]) in s |
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.
use phases(s)
to get just the list of phases
end | ||
return gs | ||
else | ||
return s[1:r, :] |
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.
So if we have S"XX iII"
this will fail to acknowledge the existence of these i
phases, right?
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.
which is fine if given what we said elsewhere about dealing only with real phases
|
||
""" | ||
For a given non-necessarily-minimal generating set represented as a tableau `s`, | ||
return the minimal generating set. |
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.
add a warning that only real phases are permitted
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.
and simplify your code accordingly
src/grouptableaux.jl
Outdated
@@ -0,0 +1,284 @@ | |||
#using QuantumClifford |
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.
clean up various comments, etc. A submitted pull request should be ready for merging.
src/grouptableaux.jl
Outdated
# `invert' arbitrary groups | ||
|
||
""" | ||
input: (generating) set of PauliOperator |
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.
Check other docstrings in the library, e.g. at http://qc.quantumsavory.org/stable/API/ and follow a similar style.
E.g. a very brief one-line header followed by a paragraph of narative text explaining what is happening. The input and output description can be just part of the text, no need for separate bullets if the function is simple.
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.
Almost all the functions in the API have the one-line header and then some code examples. For simpler functions like groupify and pauligroup, should I include the paragraph portion?
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.
yup, groupify
will not be simple to someone new to the concept
src/grouptableaux.jl
Outdated
# s = S"XXII IIXX ZIZI IZIZ" | ||
# s = groupify(s) | ||
|
||
#s = S"II YY" | ||
#s = groupify(s) | ||
|
||
#println(normalize(s)) | ||
|
||
## | ||
# Pgroup = pauligroup(2) | ||
# println(Pgroup) | ||
## | ||
|
||
# scenter = center(s) | ||
# print(scenter) | ||
# println(canonicalize!(s)[1:,]) | ||
|
||
# function center(s::Stabilizer) # centerify? centrify? | ||
# # # returns pairs of non-commuting generators | ||
# end | ||
|
||
# function |
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.
clean up
src/grouptableaux.jl
Outdated
|
||
|
||
|
||
function contract(s::PauliOperator, subset) |
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.
probably needs a better name and a docstring
src/grouptableaux.jl
Outdated
return Stabilizer([P for P in contract.(𝒮, (subset,)) if !isnothing(P)]) # make more efficient | ||
end =# | ||
|
||
function delete(s::PauliOperator, subset) |
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.
Should be the standard function Base.deleteat!
, not a new function delete
. If it is being implemented, it should probably go in the file where setindex
and getindex
are defined.
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.
getindex and setindex are defined in densecliffords.jl, paulioperators.jl, and QuantumClifford.jl. Should I place this function in QuantumClifford.jl as it seems to be the "base" file?
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.
probably in paulioperators
src/grouptableaux.jl
Outdated
# s1 = S"XIXIZIII ZZIIXIII ZIZIIXII IZIZIIXI ZIIIXIIX IIXIIZII IIIXIIZI IXIXZIIZ" | ||
# s1 = groupify(s1) | ||
|
||
# println("______!!!") | ||
# s1 = delete(s1, [6, 7, 8]) | ||
# s1 = contract(s1, [5]) | ||
|
||
|
||
# s2 = S"XIXIZIII ZZIIXIII ZIZIIXII IZIZIIXI ZIIIXIIX IIXIIZII IIIXIIZI IXIXZIIZ" | ||
# s2 = groupify(s2) | ||
|
||
# println("______!!!") | ||
# s2 = contract(s2, [6, 7, 8]) | ||
# s2 = contract(s2, [5]) | ||
|
||
|
||
# println(canonicalize!(s)) | ||
# println("______!!!") | ||
|
||
# logical operators | ||
# s = contract(s, [6, 7, 8]) | ||
# s = delete(s, [5]) | ||
#println(canonicalize!(s)) | ||
|
||
|
||
# s_contract = contract(s, [1,2]) | ||
|
||
|
||
# println("-----") | ||
|
||
# s = S"XZ ZX" | ||
# s = groupify(s) | ||
|
||
# s = S"XZZ ZXZ ZZX" | ||
# s = groupify(s) | ||
|
||
# s = Stabilizer(wheel_graph(6)) | ||
# s = groupify(s) | ||
|
||
# deleted = delete(s, [1]) | ||
# println(canonicalize!(deleted)) | ||
# println("_---") | ||
# contracted = contract(s, [1]) | ||
# println(canonicalize!(contracted)[1:4]) | ||
|
||
# println("---") | ||
# using QuantumClifford.ECC | ||
|
||
# println(canonicalize!(parity_checks(Perfect5())[:,[1, 2, 3, 5, 4]])) | ||
# println(typeof(groupify(S"XZ ZX"))) | ||
|
||
|
||
# # md = MixedDestabilizer(s); groupify(copy(vcat(stabilizerview(md), logicalxview(md), logicalzview(md)))) x |
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.
clean up
CHANGELOG.md
Outdated
Implemented get_generating_set function to find minimal generating set of a group of Paulis | ||
Implemented normalize function to find all Paulis that commute with a given set of paulis | ||
Implemented center function to find all Paulis within a set that commute with all other elements of that set | ||
Added united tests for all four functions |
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.
You can leave CHANGELOG unchanged to avoid merge conflicts until we are done with the reviews. You probably want to rebase this on master:
git checkout master
git pull # potentially with specifying the upstream remote
git checkout some_group_functions
git rebase master
Adding a see also from trace! to delete_columns causes the docs to fail to build, I think because the project_trace_reset.jl docs get created before the group_tableaux.jl ones. Additionally, the build fails, but it seems to be an error with the second part, QuantumSavory, and not QuantumClifford. Let me know if this is not the case and I will debug it. |
CHANGELOG: |
@Krastanov re-pinging for review as it is now fully complete but a review request is currently active. |
end | ||
end | ||
if commutes == 0 |
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 does not seem to match the description. I would have expected only +X
to be returned.
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 made a few documentation changes. Generally, the documentation should say what something is and why it exists. It should say "how" only if there is something particularly non-trivial and only if the previous two questions are already fully addressed. Frequently that "how" might be just in the code comments. If the "how" is mostly obvious from the code, it should probably not even be a comment. The reasoning behind this is two-fold: (1) the user probably does not care exactly how something is implemented, rather they care what it is; (2) the implementation might change without anyone remembering the update the documentation.
I do have one question for clarification on the contract function. Otherwise the rest looks great.
I have removed the groupify functionality from contractor. PR should be all good. |
Thanks, @IsaacP1234 , this was great! @KDGoodenough , let's see what you are planning to use this for! |
Co-authored-by: Kenneth Goodenough <[email protected]> Co-authored-by: Stefan Krastanov <[email protected]> Co-authored-by: Stefan Krastanov <[email protected]>
Implemented groupify function to find group of Paulis generated by a generating set.
Implemented get_generating_set function to find minimal generating set of a group of Paulis.
Implemented normalize function to find all Paulis that commute with a given set of paulis.
Implemented center function to find all Paulis within a set that commute with all other elements of that set.
Added united tests for all four functions.