Bicycle and Unicycle codes: Fixing doc and method errors #425
Conversation
Added multiple methods: code_generation: - Methods for creating Bicycle and Unicycle codes - Methods for assembling CSS codes - Utility methods for working with codes code_evaluation: - Methods for evaluating codes with a belief propagation decoder
Added the following dependencies: CairoMakie LDPCDecoders SparseArrays Statistics
During discussion with Anthony, he mentioned that he would add his code himself later on
- Fixed naming and organization issued identified. - Limited exports - Added additional methods to easily create unicycle and bicycle codes
- Fixed issues with typing in bicycle gen methods - Fixed issue with incorrectly copied methods for unicycle generation - Fixed imports for Nemo methods
Fixed bugs preventing the generation of Bicycle and Unicycle codes
Remove LDPCDecoders import Co-authored-by: Stefan Krastanov <[email protected]>
Remove LDPCDecoders import Co-authored-by: Stefan Krastanov <[email protected]>
Updated use case description Co-authored-by: Stefan Krastanov <[email protected]>
Remove unused import Co-authored-by: Stefan Krastanov <[email protected]>
Update use case of Bicycle matrix Co-authored-by: Stefan Krastanov <[email protected]>
Updated CSS struct to use Hx and Hz instead of just the full tableau form. Updated CSS methods to work with the new struct form
- Added more jldoctests for methods - Updated method descriptions with updated names - Fixed broken reduce_unicycle method - Added doc links to referenced papers
|
I converted it to draft to make it a bit easier for me to manage my review queue. |
Focused on updating the struct docstrings to be more descriptive of what the code is/does. Additionally cleaned up miscellaneous aspects of the code and remove unused function in simple_sparse_codes.jl
|
Hi Stefan, thanks for the review! I went back through and updated a few things with a focus on the function headers. Please let me know if there are any other things that you'd like me to check for! |
…et_gen Made some small changes to the structure of the code, but primarily edited bicycle_set_gen. Added information about how the method works, what problems one can encounter, and what it returns.
|
Hi Stefan, I added a little description to the index generation method covering some information I thought was relevant! Please let me know if you think anything is missing from the description :) |
There was a problem hiding this comment.
Finally got around to reviewing, thank you for your patience!
I think this is the last round of review -- this is very close to merging.
Do you think you would have a chance of looking into the change requests? I would be interested in adding this code to the wiki
| struct Bicycle | ||
| "Width of array, should be >= 2. Equal to number to number of physical qubits (N)" | ||
| n::Int | ||
| "Height of array, should be >= 2 and a multiple of 2. Equal to number of parity checks (M)" | ||
| m::Int | ||
| end |
There was a problem hiding this comment.
I think my main remaining request for change is related to the distinction between a structure and a function.
An instance of the type Bicycle should be a fully defined code where quickly and deterministically one can get a parity check matrix. The "quick" part is nice to have, but "deterministic" is mandatory.
Here, I think this is not deterministic. On the other hand the output of bicycle_set_gen seems to be all you need to be able to deterministically do the other steps. Thus I would suggest having set be an entry here, basically the way you have already done it for Unicycle.
That way both Bicycle and Unicycle will be fully defined codes by their fields, without any randomness. And the documentation can spell out the difference in what set needs to be. For Bicycle it needs to just be a set of integers (right? or maybe there are more constraints) while for Unicycle it has to be perfect difference set?
And in the docstring you can mention "we have a heuristic for finding useful sets in the function yadayada"
| return parity_checks(CSS(rusc, rusc)) | ||
| end | ||
|
|
||
| """Takes an untrimmed bicycle matrix and removes the row which keeps the spread of the column weights minimal. |
There was a problem hiding this comment.
Is this something you invented or something from the paper? In either case, explain why it is needed. If it is from the paper, give a specific reference where in the paper it is.
| return comp_matrix | ||
| end | ||
|
|
||
| """Takes an untrimmed unicycle matrix and removes linearly dependent rows. |
There was a problem hiding this comment.
same question about where this is coming from and why is it necessary
| return comp_matrix | ||
| end | ||
|
|
||
| """ |
There was a problem hiding this comment.
same question about provenance and invention and reference
There was a problem hiding this comment.
if this is your own thing, you should brag about it a bit more loudly :D
| while size(m)[1] > r | ||
| tm = vcat(m[1:i-1,:], m[i+1:end,:]) | ||
| tr = LinearAlgebra.rank(matrix(rrzz, tm)) | ||
| if(tr == r) | ||
| m = tm | ||
| if(size(m)[1] == r) | ||
| break | ||
| end | ||
| else | ||
| i += 1 | ||
| end | ||
| end |
There was a problem hiding this comment.
This is an n^4 algorithm, right? n^3 for the Gaussian elimination in rank and then another factor of n for the loop?
Add something to the documentation saying that "a naive implementation of row-reduction currently bottlenecks the use on very large codes".
| std_min = Inf | ||
| for i in 1:m | ||
| t_H0 = vcat(H0[1:i-1, :], H0[i+1:end, :]) | ||
| std_temp = std(convert(Array, sum(t_H0, dims = 1))) |
There was a problem hiding this comment.
The convert(Array, is pretty suspicious. Do you need it?
Hi! I finally got around to addressing the problems with my previous commits. I made a few changes:
reduce_unitaryIf you have the chance to review this and let me know if you want me to fix anything else, that would be great! Thanks so much :)
I was unable to open the previous pull request so I made a new one here.