-
Notifications
You must be signed in to change notification settings - Fork 49
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
Error correction methods for creating Bicycle and Unicycle codes + belief decoder code evaluation #195
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
This is pretty exciting! I left a bunch of comments in, but they are mostly on coding conventions, not on the substance of the PR. I would suggest putting the CCS things in a separate file called We should think of adding some tests, but we can discuss this later. |
- Fixed naming and organization issued identified. - Limited exports - Added additional methods to easily create unicycle and bicycle codes
I did some minor cleanup along the lines of the comments I just posted. Make sure you pull locally before you continue modifying. Also, it is helpful to always create a separate branch when making modifications. Currently all of this seems to be on your master branch which makes it difficult for you to experiment with multiple things at the same time. I think there are still some things not functional in simple_sparse_codes.jl When you address a comment, please also click "resolve" on it, as it makes it easier for me to review. I will mark this as a draft. When you are ready for the next round of reviews, just mark it back to not-a-draft. |
- 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
--------- Co-authored-by: Stefan Krastanov <[email protected]> Co-authored-by: Stefan Krastanov <[email protected]>
* add compat downgrader * bump compats for quantuminterface and quantumopticsbase * bump compats for docstringextensions * bump compats for graphs * bump nemo to 0.38 * nemo fixes
Ben, I just now noticed that you had actually marked this as ready for review for quite a while. I apologize for my oversight! Could you click "resolve conversation" on the comments you have addressed in the follow-up edits? That would make it much easier for me to finish the review. |
Hi Stefan, I apologize for not informing you that I pushed an update. I believe that my most recent commits should resolve any problems. I also tested my code to ensure it works, but please let me know if I've made any glaring errors! |
Bumps [dawidd6/action-download-artifact](https://github.com/dawidd6/action-download-artifact) from 2 to 3. - [Release notes](https://github.com/dawidd6/action-download-artifact/releases) - [Commits](dawidd6/action-download-artifact@v2...v3) --- updated-dependencies: - dependency-name: dawidd6/action-download-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 3 to 4. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v3...v4) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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 added a few more comments. This will also need tests before we can merge it. In particular, tests for whether the generated codes are valid.
One possible type of tests is to just check exactly for the parity check matrix of known small codes.
Another type of tests to add is tests for random large codes verifying they produce valid tableaux.
I also noticed that you are working on your master branch. That makes work generally more difficult, as it is cumbersome to keep things in sync with the main repository. |
@amicciche , skim through the CSS definition that Ben is making here. Just making sure you guys are not duplicating work that is done by the other. |
My definition of a CSS struct was the bare minimum to get the idea of the functionality implemented quickly. In other words, it was sort of a functional placeholder. Here it is in its entirety: struct CSS; tableau; cx; cz; end Ben's version seems to be a bit more detailed and fleshed out, and could easily just replace my version. I do have some code that was not included in my pull request that coverts the matrices into a stabilizer tableau, and perhaps could be part of the CSS struct? |
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
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumClifford.jl/QuantumClifford.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumClifford.jl/QuantumClifford.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumClifford.jl/QuantumClifford.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumClifford.jl/QuantumClifford.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumClifford.jl/QuantumClifford.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumClifford.jl/QuantumClifford.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumClifford.jl/QuantumClifford.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumClifford.jl/QuantumClifford.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumClifford.jl/QuantumClifford.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumClifford.jl/QuantumClifford.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
|
Thank you for the review! I have implemented all the changes you requested. Additionally, I will be using new branches for all future commits. Thank you for that recommendation as well! |
Added multiple methods:
code_generation:
code_evaluation:
ECC.jl was modified to include the required imports for the two added files.