-
Notifications
You must be signed in to change notification settings - Fork 36
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
Add custom attribute types to the decoding graph #334
Add custom attribute types to the decoding graph #334
Conversation
Tests won't run because of circular import issue
This patch adds support for the new Node everywhere, such that it passes all tests. It also moves DecodingGraph to the analysis module due to a circular dependency issue.
And lint and black
And lint and black
…asopeduzzi/qiskit-qec into decoding-graph-custom-types
74ed5e7
to
3c732a5
Compare
Does this look good? @dsvandet @quantumjim |
Having classes for nodes and edges is a great idea. They look well implemented. And the tests pass! I'll go through my QEC Summer School lecture notes today using this branch, see what would need to be changed there and whether I find any fatal flaws. |
@@ -2,9 +2,9 @@ | |||
|
|||
# This code is part of Qiskit. | |||
# | |||
# (C) Copyright IBM 2019. |
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 think 2019-2022 is better, reflecting all the years this file has been changed.
Are not considered when comparing nodes. | ||
""" | ||
|
||
def __init__(self, qubits: List[int], index: int, is_boundary=False, time=None) -> None: |
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.
Could we make qubits
optional? Sometimes we might make decoders for qudit codes, for example, so this won't always be relevant.
test/union_find/test_clayg.py
Outdated
|
||
def test_error_rates(self): | ||
""" | ||
Test the error rates using some ARCs. |
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 you use standard repetition codes rather than ARCs
atypical_nodes: List[int] # List[node_index] | ||
|
||
|
||
class ClAYGDecoder(UnionFindDecoder): |
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.
Maybe it's better for ClAYG to have its own PR?
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 accidentally created the custom-type-branch on top of the clayg branch, so it has some changes on it which depend on the ClAYG. I can try to do a rebase onto main, but it will probably be a mess.
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 good in general.
One issue is that Decodoku still uses the old nodes. I don't see this is a big problem, though, since it is inherently isolated and does its own thing. We can address this one day, but it doesn't need to be today.
9d94e44
to
3c443f5
Compare
@quantumjim I have closed this one, because I have separated the branches for the clayg decoder and the decoding graph stuff. I will create a PR for the clayg when it's ready. |
Summary
This patch adds custom Node and Edge classes for the attributes of the rustworkx decoding graph. It adds support for these classes everywhere, such that the tests pass.
Because both the decoders and the circuits are now dependent on the decoding_graph.py file, it was moved to the analysis submodule.
This PR also adds a novel decoder to qiskit-qec. It is called the Cluster-As-You-Go-Decoder, because of how it works:
It reduces the search space of the union find decoder from two dimensions to three by adding syndrome nodes separated by time to a two-dimensional slice of the decoding graph. It performs about the same in terms of decoding performance as the union find decoder, but can be easily distributed using just O(n^2) resources.
Details and comments
Also added some tools to temp_graph_utils.py to cache decoding graphs to files for testing.