Lifted and lifted product codes

[royess]

Add lifted codes, lifted product (LP) codes, and group ring functionality, which is essential for these codes.

Note: The added LP codes trigger errors in syndrome circuits, as reported in #306. So, decoding tests only include commutativity checks at the moment. For the same reason, I also skip LP codes in all_testablable_code_instances.

[codecov]

Codecov Report

Attention: Patch coverage is 60.25641% with 62 lines in your changes missing coverage. Please review.

Project coverage is 82.92%. Comparing base (59e399d) to head (53322b5).
Report is 24 commits behind head on master.

Files with missing lines	Patch %	Lines
src/ecc/group_ring.jl	59.01%	50 Missing ⚠️
src/ecc/codes/classical/lifted.jl	52.94%	8 Missing ⚠️
src/ecc/codes/lifted_product.jl	76.47%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #312      +/-   ##
==========================================
- Coverage   82.97%   82.92%   -0.05%     
==========================================
  Files          61       64       +3     
  Lines        4023     4171     +148     
==========================================
+ Hits         3338     3459     +121     
- Misses        685      712      +27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[royess]

• The failure of the downgrade test should be related to Nemo's degrading from 0.4.5 to 0.4.2.

  In Nemo 0.4.5, FqFieldElem is a subtype of RingElement, which matches the following function call in my codes.

  base_ring_type(::Type{PermGroupRing{T}}) where {T<:RingElement}

  When tested with Nemo 0.4.2, this function call breaks. Possibly, it is the case that FqFieldElem was not a subtype of RingElement back then.

• The nighly tests fail because of a bug in the external PyBeliefPropOSDecoder, which looks irrelevant to the added implementation.

  belief prop decoders, good for sparse codes: Error During Test at /home/runner/work/QuantumClifford.jl/QuantumClifford.jl/test/test_ecc_decoder_all_setups.jl:89
    Got exception outside of a @test
    Python: Julia: MethodError: no method matching pyjlarray_array__array(::Nothing)
    The function `pyjlarray_array__array` exists, but no method is defined for this combination of argument types.
    
    Closest candidates are:
      pyjlarray_array__array(::AbstractArray)
       @ PythonCall ~/.julia/packages/PythonCall/S5MOg/src/JlWrap/array.jl:269
    
    Stacktrace:
      [1] _pyjl_callmethod(f::Any, self_::Ptr{PythonCall.C.PyObject}, args_::Ptr{PythonCall.C.PyObject}, nargs::Int64)
        @ PythonCall.JlWrap ~/.julia/packages/PythonCall/S5MOg/src/JlWrap/base.jl:61
      [2] _pyjl_callmethod(o::Ptr{PythonCall.C.PyObject}, args::Ptr{PythonCall.C.PyObject})
        @ PythonCall.JlWrap.Cjl ~/.julia/packages/PythonCall/S5MOg/src/JlWrap/C.jl:63
      [3] PyObject_CallObject(x1::Ptr{PythonCall.C.PyObject}, x2::Ptr{PythonCall.C.PyObject})
        @ PythonCall.C ~/.julia/packages/PythonCall/S5MOg/src/C/pointers.jl:297
      [4] macro expansion
        @ ~/.julia/packages/PythonCall/S5MOg/src/Core/Py.jl:132 [inlined]
      [5] pycallargs(f::PythonCall.Core.Py, args::PythonCall.Core.Py)
        @ PythonCall.Core ~/.julia/packages/PythonCall/S5MOg/src/Core/builtins.jl:212
      [6] pycall(f::PythonCall.Core.Py, args::Vector{Bool}; kwargs::@Kwargs{})
        @ PythonCall.Core ~/.julia/packages/PythonCall/S5MOg/src/Core/builtins.jl:230
      [7] pycall
        @ ~/.julia/packages/PythonCall/S5MOg/src/Core/builtins.jl:220 [inlined]
      [8] (::PythonCall.Core.Py)(args::Vector{Bool})
        @ PythonCall.Core ~/.julia/packages/PythonCall/S5MOg/src/Core/Py.jl:339
      [9] decode(d::QuantumCliffordPyQDecodersExt.PyBeliefPropOSDecoder, syndrome_sample::SubArray{Bool, 1, Matrix{Bool}, Tuple{Int64, Base.Slice{Base.OneTo{Int64}}}, true})
        @ QuantumCliffordPyQDecodersExt ~/work/QuantumClifford.jl/QuantumClifford.jl/ext/QuantumCliffordPyQDecodersExt/QuantumCliffordPyQDecodersExt.jl:74
     [10] batchdecode(d::QuantumCliffordPyQDecodersExt.PyBeliefPropOSDecoder, syndrome_samples::Matrix{Bool})
        @ QuantumClifford.ECC ~/work/QuantumClifford.jl/QuantumClifford.jl/src/ecc/decoder_pipeline.jl:20
  

[royess]

@Krastanov please help review this PR, thanks!

[Krastanov]

This is shaping up neatly, thank you!

I left in a lot of comments but the overarching concerns are:

• most importantly, better documentation, and potentially reworking the user-facing API, as currently a user will have a lot of difficulty creating a code
• a lot of custom code that might break if Nemo has a breaking release (they do that often)
• better test coverage

For documenting fields of structs, I would suggest using the docstringhelpers package. See for instance here: https://github.com/QuantumSavory/QuantumSavory.jl/blob/master/src/ProtocolZoo/switches.jl#L139-L157

[Krastanov]

We should not be exporting all these fairly internal utilities, at least not until a few more versions come out and we are sure we are sticking to them.

[Krastanov]

I am surprised this needs a general function. Is there any situation where we would need something besides the default?

[royess]

I leave this because the default permutation (cyclic) matrix representation we provided is not necessarily the most efficient one. There are cases we can find a representation that need fewer qubit number. So I want to allow the customization.

[Krastanov]

Could you give more specific examples? Permitting an arbitrary function in a situation like this is generally a sign of missing abstraction, especially in a language like Julia where you have access to multimethods.

[royess]

Let's consider a simple case: D4 group. The wiki.

In our default way, the representation will be 8x8 matrices, where 8 is the group's order.

But we can find a 4x4 matrix representation for the group. We can start with its representation on integer matrix space:

What we need is binary matrices; therefore, we can replace "1" with the 2x2 identity matrix, "0" with the 2x2 zero matrices, and "-1" with the Pauli X matrix. After replacement, we will get a 4x4 matrix representation.

[Krastanov]

Examples like this should probably be part of the documentation. Otherwise the user would be pretty confused how to approach these extra capabilities.

[Krastanov]

Why is this printing as a dictionary?

[Krastanov]

what it means to call this on a dictionary

[Krastanov]

I think a lot of my concerns expressed elsewhere can be addressed by being a bit more detailed with what these are, something like an extra paragraph expounding upon: these are the lifted product codes (cite) which are a generalization of the general bicycle codes (cite) which are a generalization of the bicycle code (cite).

Then we can have something like: the bicycle code is defined by such and such set and if you have that set you can convert it to the necessary representation and use it here by doing so and so.

The generalized bicycle code is defined by a circulant matrix and you can use a circulant matrix to create a code here by doing yada yada

Finally, the most general form is yada yada where you need to provide so and so.

One of my worries is that currently I do not know how to use what you have implemented to set up the generalized bicycle codes from https://quantum-journal.org/papers/q-2021-11-22-585/pdf/

See footnote 2 on page 2 of https://quantum-journal.org/papers/q-2021-11-22-585/pdf/

[Krastanov]

related to my post from a minute ago: I think this is an important question

[royess]

Regarding the concern you have on generalized bicycle codes, I think we can construct them using 1x1 matrices. For example, we consider the first example in Appendix B (page 20) of https://quantum-journal.org/papers/q-2021-11-22-585/pdf/,

using my implementation,

l = 127
R = PermutationGroupRing(GF(2), l)
x = R(cyclic_permutation(1, l))
A = [1 + x^15 + x^20 + x^28 + x^66]
B = [1 + x^58 + x^59 + x^100 + x^121]
LPCode(LiftedCode(A), LiftedCode(B))

The current documentation is indeed so bad for these special cases. I will improve the document by adding these cases.

[Krastanov]

Looking at the end of this list of changes, it seems like you are imagining the user doing a couple of different things to generate codes, but it does not seem like the starting point is really this matrix.

Can we structure it in a way that is simpler, so that the user just gives what would be the natural object for them to work with. Especially looking at the test file, it does not seem like you expect most users to start with such a matrix. Here are a few suggestions, probably some of them are bad, maybe all of them are bad, but some discussion of making this API easier to use is needed:

• maybe a constructor that just takes a dictionary of permutation and does all the conversions behind the scene
• a constructor that just takes the circulant matrices or maybe a separate struct for simpler instances of these codes (bicycle codes and generalized bicycle codes)

[Krastanov]

related to my post from a minute ago: I think this is an important question

[royess]

In #312 (comment), I proposed two new constructors: one from Matrix{Perm}, the other from Matrix{Int}.

As for the suggestion here,

maybe a constructor that just takes a dictionary of permutation and does all the conversions behind the scene

I agree it will be good to have this.

a constructor that just takes the circulant matrices or maybe a separate struct for simpler instances of these codes (bicycle codes and generalized bicycle codes)

I think taking circulant matrices is mathematically the same as taking Matrix{Int}, the meaning of which is explained in #312 (comment). And the latter would be more efficient.

As for 2GBA codes, including bicycle codes and generalized bicycle codes, the only difference is the matrix will be 1x1. It looks to me that having something like LiftedCode(::Perm) might be slightly confusing, although more convenient.

If we make a separate struct, then seemingly everything other than new constructors will be the same as LiftedCode. We can choose to re-implement and simplify the production construction. But it is not very appealing to me.

Perhaps it makes more sense to provide some functions (but not make them constructors) to construct these simpler codes, which can still be in the LPCode type.

[Krastanov]

You are already hardcoding the use of your PermGroupRingElem, so we can probably be a bit neater about repr. Instead of asking for a repr here, you can ask for a new method of permutation_repr that specializes on the group type.

[royess]

(This seems to be related to your comments on the repr in the docstring. There #312 (comment), I left a comment about why I think other-than-default representations will be useful.)

Regarding the suggestion here, I think it assumes that users would define their own group ring type in analogy to PermGroupRing, which can be done in principle but requires too much work. I do not expect most users to do that. And if they still use our PermGroupRing, then they have to redefine the method and cover the default one, which looks more confusing to me. Also, there is the possibility that one wants to use different representations for the same group type in different codes. In that case, the methods will conflict.

Instead, the current implementation assumes that users need to customize matrix representation for their own group and still want to use our PermGroupRing. The logic behind this is that one can always use the permutation group to represent any other group, which is the way OScar.jl does, for example. In this way, PermGroupRing can represent the specialized group, and we ask for a customized function that may better deal the specialized group.

The general function here also looks bad for me. But with these considerations, I do not have better ideas. I would appreciate your input on this.

Maybe a way is to define parity_checks(c::LiftedCode; repr) and keep at least the LiftedCode type clearly defined if we accept that a code can have various parity checks.

[Krastanov]

My reasoning starts from the following point: you are asking the user of this structure to provide an "unlifted" matrix and a lifting function, but the only thing you do with those two entities is to calculate the "lifted" matrix. That is something the user could have already done themselves and then they just provide us with the lifted matrix directly. We do not do anything sophisticated with the abstraction created here, so that abstraction probably should not exist (because the abstraction does not simplify anything for the user -- they still need to know what the unlifted matrix is and what the lifting function is, and now they also need to learn this library-specific abstraction).

Obviously, my description is a bit willfully negative, but this is a design principle on which I believe we should be leaning hard.

Then there is a secondary issue that by just asking for a function without specifying what interface that function should have, we are making this too unconstrained and error-prone.

Lastly, I am confused when you are saying that the user will define their own type in analogy to the one you have created. If they do so, they would not be able to use this structure, because the matrix element type is hardcoded.

Defining a parity_checks(c::LiftedCode; repr) would be a bit of a "pun" in Julia parlance. It creates a method for a function that does not follow the structure expected from all other methods, making composability a bit more difficult.

I am still leaning towards avoiding using the new type you have created (I have a lot of questions related to it elsewhere through this PR -- answering them would help me understand why such a custom type is necessary, instead of just using Nemo/Oscar as is). Independently of whether we are relying on the new custom type, setting up a lift function and asking the user to define methods for it seems cleaner to me (with a default method provided by us).

But I might be completely wrong about this. I would not know that until I have more information about how the user is expected to use this code. I have a few more questions in this PR that when answered I will have a better idea. Look at the questions about "how would the user use this to represent common types of codes". I will mark them up again now.

[royess]

Thanks for these comments! I replied to the questions that you marked (and #312 (comment) and #312 (comment)), which hopefully clarified some important points that I have not documented well.

For a short answer to "why such a custom type is necessary" is that we need something with multiplication and addition, where the multiplication is defined by permutation group.

Before writing my type, I checked the types in Nemo and Oscar. But I did not find any of them suitable. There, they have rings that do not have group multiplication and the permutation group that does not have addition. What I have implemented is basically a composition of both.

[Krastanov]

I just noticed this, but why does LPCode have a repr function if the LiftedCode structures inside of it have one already as well?

[Krastanov]

Thanks for all the answers you have provided -- they help in wrapping my head around your work. In terms of TODOs, besides the things you are already taking care of, I think we need the following:

• Make a draft pull request to the Nemo folks with the code you have created for the permutation representations here, to get their opinion on your approach. No expectation to have such a PR merged in the near term, but it would be valuable to engage more properly with that community, given how heavily we are relying on their functionality.
• Add to the docstring of the permutation object you have created a section along the lines of "why do we need this and why is it not upstreamed to Nemo". I think I am mostly convinced by the reasons you have given, but keeping a record of these reasons for future developers would be very useful. This is related to the first point.
• Make simple intuitive ways to construct the historical precursors of the general lifted product codes (bicycle and generalized bicycle) with documentation (and references to papers) that explain how the generalization works
• Make all the constructors you enumerated in Lifted and lifted product codes #312 (comment) (this is related to the previous point)
• Document all of these constructors (potentially including examples from papers)
• Re-examine the choice of internal fields for the code types you have created -- the hardcoded matrix type and the general "representation" function. I personally do not have better ideas right now, but after having the previous 3 points completed, I imagine it would be easier to figure out whether there is a better option here.

[Krastanov]

You mentioned that you have some pros and cons in mind about whether to make helper functions, or constructors, or new types for all these "simpler" versions of LP codes -- please proceed with what you believe is most reasonable and do not worry if I happen to use the word "constructor" when you were thinking about a "helper function"

[royess]

Thanks for your detailed suggestions!

I think I am clear about these TODOs and plan to complete them in the next couple of days.

[Krastanov]

closing now that #356 is merged