Ldpc v2 #18
Ldpc v2 #18
Conversation
cpp_example/main.cpp
Outdated
| @@ -0,0 +1,13 @@ | |||
| #include <iostream> | |||
cpp_test/TestFlipDecoder.cpp
Outdated
|
|
||
| cout<<endl; | ||
|
|
||
| for(int i = 2; i<8; i++){ |
There was a problem hiding this comment.
I believe for indexing std::size_t is best to use as loop variable type
cpp_test/TestFlipDecoder.cpp
Outdated
|
|
||
| auto decoding = flipD->decode(syndrome); | ||
|
|
||
| cout<<"Error: "; |
There was a problem hiding this comment.
should we leave in all cout statements in the release?
cpp_test/TestGf2Dense.cpp
Outdated
| return csr_matrix; | ||
| } | ||
|
|
||
| // TEST(gf2dense, hamming_code){ |
There was a problem hiding this comment.
what about all this outcommented code?
cpp_test/TestGf2Linalg.cpp
Outdated
|
|
||
| } | ||
|
|
||
| double sparsity = 0.01; |
There was a problem hiding this comment.
unsure if blobal variables adhere to the best practices here.
cpp_test/TestSoftInfo.cpp
Outdated
| #include "osd.hpp" | ||
|
|
||
| using namespace std; | ||
|
|
There was a problem hiding this comment.
lots of code commented out here
cpp_test/TestUnionFind.cpp
Outdated
| using namespace ldpc::uf; | ||
|
|
||
| // TEST(UfDecoder, single_bit_error) { | ||
|
|
| print(csv_string, file=output_file) | ||
|
|
||
|
|
||
| if __name__ == "__main__": |
There was a problem hiding this comment.
is a main function needed here?
python_test/test_bp_decoder.py
Outdated
|
|
||
|
|
||
|
|
||
| if __name__ == "__main__": |
python_test/test_code_util.py
Outdated
|
|
||
| for i in range(3,10): | ||
|
|
||
| H = hamming_code(i) |
There was a problem hiding this comment.
I guess using pytest.fixture would be the best practice to generate test objects used (also those used across different tests)
| @@ -29,23 +63,33 @@ def decode_via_files( | |||
| ) -> None: | |||
| self.dem = stim.DetectorErrorModel.from_file(dem_path) | |||
| self.matrices = detector_error_model_to_check_matrices(self.dem) | |||
| self.bposd = BpOsdDecoder(self.matrices.check_matrix, error_channel=list(self.matrices.priors), max_iter=5, bp_method="ms", ms_scaling_factor = 0.625, schedule="parallel", osd_method = "osd0") | |||
| self.bposd = BpOsdDecoder(self.matrices.check_matrix, | |||
There was a problem hiding this comment.
we might save some time and use a common base class instead of a specific decoder here. Making this more general allows to simply reuse and not require a separate sinter_[decoder name]_ for every decoder.
Something like a SinterDecoderBase would probably be the best solution?
There was a problem hiding this comment.
If I don't make a separate sinter_[decoder_name]_ for every decoder, then someone using Sinter in python can't see the the parameters the sinter_[decoder_name]_ excepts in their editor. Do you know what I mean? For a Sinter user (who I'm targeting) I think this solution is the cleanest.
There was a problem hiding this comment.
If you used named arguments that should work, no?
| stim.write_shot_data_file( | ||
| data=predictions, | ||
| path=obs_predictions_b8_out_path, | ||
| format="b8", | ||
| num_observables=self.dem.num_observables, | ||
| ) | ||
|
|
||
|
|
||
| def decode(self, syndrome: np.ndarray) -> np.ndarray: |
There was a problem hiding this comment.
having this at hand, writing a full simulator that only takes a code and a decoder/-name would be helpful. this is also more or less the same and boilerplate.
There was a problem hiding this comment.
Yes agreed, I made an example in sinter_example.py. Can you let me know what you think? To be clear, I don't want to write code that replaces Sinter and Stim, I simply want to use the BPOSD and BeliefFind as a plugin. Sinter and Stim have a full simulator that only takes a code and a decoder/-name.
|
@lucasberent Thanks for the quick review 😃 . There are two comments left which I didn't resolve yet but I did comment on. |
haven't tried it out myself yet but looks good! thanks |
Complete rewrite. Should be backwards compatible with LDPCv1. Please test this on old scripts.