-
Notifications
You must be signed in to change notification settings - Fork 27
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
Peeling union find with boundaries #29
Conversation
src_cpp/union_find.hpp
Outdated
} | ||
|
||
std::vector<uint8_t>& peel_decode(const std::vector<uint8_t>& syndrome, const std::vector<double>& bit_weights = NULL_DOUBLE_VECTOR, int bits_per_step = 1){ | ||
|
||
if(!this->is_planar_code){ | ||
throw(std::runtime_error("Peel decoder only works for planar codes. Use the matrix_decode method for more general codes.")); |
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.
Why does it not work for non-planar codes? It should work for the Toric code.
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.
What's the general name for a code that produces pair-like or pair to boundary excitations?
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.
any code with max bit_degree 2 should do that. Planar to me means non-periodic.
src_cpp/union_find.hpp
Outdated
@@ -557,7 +653,7 @@ class UfDecoder{ | |||
|
|||
invalid_clusters.clear(); | |||
for(auto cl: clusters){ | |||
if(cl->active == true && cl->parity() == 1){ | |||
if(cl->active == true && cl->parity() == 1 && cl->contains_boundary_bits == false){ |
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.
So you turn off a cluster whenever it grows into a boundary. Are there cases and UF grow orders for which this is suboptimal (or better) than growing until it becomes valid, i.e., merging with another invalid cluster? I am almost certain there are multiple cases where the resulting clusters are not equivalent.
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.
As soon as a cluster intersects a boundary, I mark it as "valid". I.e. it no longer grows. However, other clusters can grow into it and merge.
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.
As for whether this is optimal ... probably not. I've simply decided to treat boundaries as equivalent to defects in the lattice. Hence the decision to freeze growth once the boundary is hit.
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.
Thinking about this some more, shouldn't it generally be the case that we want to keep our clusters as small as possible.?
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.
Thinking about it more, I am not sure it makes a difference, yes.
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 to me in general.
I'd like to push some minor code formatting/cleanups before merging if that's ok
src_cpp/union_find.hpp
Outdated
@@ -21,6 +21,7 @@ | |||
namespace ldpc::uf{ | |||
|
|||
const std::vector<double> NULL_DOUBLE_VECTOR = {}; | |||
tsl::robin_set<int> NULL_INT_ROBIN_SET = {}; |
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'd rename those to have the prefix EMPTY instead of NULL.
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.
Yes I agree. That makes more sense
TEST(UfDecoder, peeling_with_boundaries_edge_case){ | ||
|
||
auto pcm = ldpc::gf2sparse::GF2Sparse<ldpc::bp::BpEntry>(20,41); | ||
pcm.insert_entry(0,0);pcm.insert_entry(0,5);pcm.insert_entry(0,25);pcm.insert_entry(1,1);pcm.insert_entry(1,6);pcm.insert_entry(1,25);pcm.insert_entry(1,26);pcm.insert_entry(2,2);pcm.insert_entry(2,7);pcm.insert_entry(2,26);pcm.insert_entry(2,27);pcm.insert_entry(3,3);pcm.insert_entry(3,8);pcm.insert_entry(3,27);pcm.insert_entry(3,28);pcm.insert_entry(4,4);pcm.insert_entry(4,9);pcm.insert_entry(4,28);pcm.insert_entry(5,5);pcm.insert_entry(5,10);pcm.insert_entry(5,29);pcm.insert_entry(6,6);pcm.insert_entry(6,11);pcm.insert_entry(6,29);pcm.insert_entry(6,30);pcm.insert_entry(7,7);pcm.insert_entry(7,12);pcm.insert_entry(7,30);pcm.insert_entry(7,31);pcm.insert_entry(8,8);pcm.insert_entry(8,13);pcm.insert_entry(8,31);pcm.insert_entry(8,32);pcm.insert_entry(9,9);pcm.insert_entry(9,14);pcm.insert_entry(9,32);pcm.insert_entry(10,10);pcm.insert_entry(10,15);pcm.insert_entry(10,33);pcm.insert_entry(11,11);pcm.insert_entry(11,16);pcm.insert_entry(11,33);pcm.insert_entry(11,34);pcm.insert_entry(12,12);pcm.insert_entry(12,17);pcm.insert_entry(12,34);pcm.insert_entry(12,35);pcm.insert_entry(13,13);pcm.insert_entry(13,18);pcm.insert_entry(13,35);pcm.insert_entry(13,36);pcm.insert_entry(14,14);pcm.insert_entry(14,19);pcm.insert_entry(14,36);pcm.insert_entry(15,15);pcm.insert_entry(15,20);pcm.insert_entry(15,37);pcm.insert_entry(16,16);pcm.insert_entry(16,21);pcm.insert_entry(16,37);pcm.insert_entry(16,38);pcm.insert_entry(17,17);pcm.insert_entry(17,22);pcm.insert_entry(17,38);pcm.insert_entry(17,39);pcm.insert_entry(18,18);pcm.insert_entry(18,23);pcm.insert_entry(18,39);pcm.insert_entry(18,40);pcm.insert_entry(19,19);pcm.insert_entry(19,24);pcm.insert_entry(19,40); |
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 you indicate what matrix that is? would be important for intuition and to keep the test 'alive' in the future
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.
Hi. This was just a specific test from the Python Monte Carlo tests that was failing. I ported the pcm and the syndrome over to CPP for debugging. It seems to be resolved now so this test can be deleted.
Enables decoding of surface codes with the peeling version of union find.