Polish the summer additions to the ECC API
bug
@Fe-r-oz , @nzy1997 , I finally got around to reviewing the vast array of codes you have added/reviewed over the last few months. Great work! Thank you so much for it!
As the preparation of the paper is coming up, and we should also plan on having a release for the library, I wanted to go through some cleanup chores and review. Especially given that this is a breaking release, we can use it to make things a bit more consistent. There seems to be a few unidiomatic use of structs that have been merged that would need to be fixed before the next release. Posting them here to keep track and in case you have the bandwidth to address them:
## Many structs constructors are not deterministic
A structure should be a deterministic complete representation of a code instance. There should be no randomness involved. The values of a field of a struct should completely define it.
- [x] The GoppaCode has a `seed` argument. This has to be removed as it has little to do with **what** the code instance is, rather it is implementation details on how it is implemented here.
- [x] remove `seed`
- [x] turn `GoppaCode(m, t; seed)` into `random_goppa_code(m, t)` that dispatches to `random_goppa_code(rng, m, t)` (no seeds, just rng instances)
- [x] it is not clear to me that `BivariateBicycleCode` and `GeneralizedCircuilantBivariateBicycleCode` should be separate structures or be named the way they are. We need to clarify their relationship.
- [x] the docstring for BivariateBicycleCode has a reference to `GeneralizedCirculantBivariateBicycleCode` , but it is not super clear whether this should just be a different constructor (why "generalized") -- on the other hand the docstring for GeneralizedCirculantBivariateBicycleCode explains that really well! Copying that one sentence could help.
- [x] `BivariateBicycleCode` has a weird unicode field name -- unicode is cool, but this seems like an inconsistent use as just one letter is unicode, while that particular letter also has a perfectly reasonable ascii character
- [x] I really like the description of alternatives in BivariateBicycleCode's docstring -- very thorough and well done. We should probably make a diagram to clarify it
- [x] one of them is in QuantumClifford.ECC and the other one is QECCore -- can we have both in the same location before the release
- [x] the name seems a bit inconsistent for the `GeneralizedCirculantBivariatBicycle`. Why add `Circulant`? Why remove `Code`?
- [x] `GeneralizedBicycleCode` and `generalized_bicycle_codes` -- why are there both
- [x] it seems like the struct for `GeneralizedBicycleCode` can be much simpler and just contain vectors of ints (like the inputs to `generalized_bicycle_codes`). We should move this struct out of an extension and put it directly in the base library. The extension is still necessary for the parity check functions, but the more we have in the base library the easiest it would be to extend it from other libraries
- [x] there might be good reasons to keep things in the more complicated format, but those should be explained; we can also always keep the more complicated constructors, just make sure that what is stored in the struct is the simpler representation
- [x] rename the function to `generalized_bicycle_code_as_2bga` or something similar, conveying that this is not an instance of the `GeneralizedBicycleCode` type and document that
- [x] add "see also" sections between the various Delfosse-Reichardt code implementations
- [x] Where does `CyclicQuantumTannerGraphProduct` come from? Can we add a paper reference?
- [x] is there a simpler name we can use for this?
- [x] `GallagerLDPC` should be just a function `random_gallager_ldpc` (that has a method that takes `rng` as a first argument and a method that dispatches to the default choice for global rng, no seeds)
- [x] `honeycomb_color_codes`
- [x] has a weird unicode `l` while having a normal `m`
- [x] maybe we should rename it to `honeycomb_color_code_as_2bga`
- [x] and pluralization?
- [x] `haah_cubit_codes`
- [x] capitalize?
- [x] rename to `haah_cubic_codes_as_2bga`
- [x] and pluralization?
- [x] `generalized_bicycle_codes` rename to `generalized_bicycle_codes_as_2bga` and pluralization?
- [x] `bicycle_codes` rename to `bicycle_codes_as_2bga` and pluralization?
- [x] Do we need `QuantumTannerGraphProduct` when we already have the `hgp` function? It seems better to me to remove it and just have a single way to perform that operation. The high-quality documentation that was added for `QuantumTannerGraphProduct` can just be moved to the `hpg` docstring
- [x] `HomologicalProductCode` -- why is the field a vector of "weird" matrices. Can it be simpler, potentially not depend on Oscar
- [x] `DoubleHomologicalProductCode` -- why is the field an abstract matrix. Is a matrix of booleans general enough?
- [x] `random_TillichZemor_code` needs a reference. Is this something that you made up and named after them or is this exact procedure something they suggested in a paper (adding a reference to that paper would help)
- [x] "D-dimensional surface and toric codes"
- [x] there is a TODO in `code_n` -- is it broken or wrong currently?
---
`GeneralizedToricCode` and `GeneralizedHyperGraphProductCode` and "La-cross" look good
---
- [x] in function names, pick consistent capitalization for names of people; e.g. `random_gallager..`, `random_TillichZemor...`, `haah_...` do not stick to a similar style
- [x] fix the pluralization of all function names (should they end on `code` or `codes`)
- [x] decide on a standard for type names -- should they end on `Code` or not -- they are not particularly consistent right now
---
- [ ] The use of Nemo for a few interface functions in QECCore needs proper MethodError hints
---
- [x] when randomness is necessary in a reproducible way, use an rng instance as done in `Base.rand`, not a seed, as it provides a more universal way to prepare rng algorithms
- [x] all references to other functions and structs should be linked with `@ref`
- [x] as much as possible should be in the base library, not the extensions
- [x] for each struct that ends up remaining in the extension we should make sure that it renders properly in documentation
- [x] consistency in unicode use -- in particular, a fancy unicode `ℓ` is used in a lot of places that probably should just use ascii `l` unless there are good reasons
---
- [ ] move all codes to QECCore -- this is here just as a note for me, it is not really something that should be blocking a release
---
Of note, using `seed` for a struct is not just a bad idiom counter the point of what a type is supposed to represent. It is also non-portable / reproducible. You do not have a guarantee and `MersenneTwister(seed)` will always act the same way on all versions of julia.
关闭于 2025-12-31 17 条评论