Add information set function #41
Conversation
There was a problem hiding this comment.
I would like to add a random_information_set function. I imagine this is something like... Take gen mat G, perm = shuffle(1, C.n), G_perm = G[:, perm], repeat alg, return cols wrt original matrix using invperm. (You should verify this makes sense conceptually.)
| @@ -396,6 +396,19 @@ end | |||
| ############################# | |||
| # general functions | |||
| ############################# | |||
| function information_set(C::AbstractLinearCode) | |||
| """ | |||
There was a problem hiding this comment.
docstrings need to go directly above the function in Julia
| nonpivot_inds = _non_pivot_cols(C.G, :nsp) | ||
| pivot_inds = [x for x ∈ 1:C.n if x ∉ nonpivot_inds] | ||
| end | ||
| return pivot_inds, C.G[:, pivot_inds] |
There was a problem hiding this comment.
Idk how meaningful it is to return the submatrix. Since the user can easily get those, I'd suggest we only return the columns.
| pivot_inds=[x for x ∈ 1:C.n] | ||
| else | ||
| nonpivot_inds = _non_pivot_cols(C.G, :nsp) | ||
| pivot_inds = [x for x ∈ 1:C.n if x ∉ nonpivot_inds] |
There was a problem hiding this comment.
Nothing wrong with this, but we've stuck with the Julia notation of using in during the for loop and then the symbol \in for data structures.
| Returns the indexs of the pivot columns and an information set which | ||
| is the submatrix of G given by these columns. | ||
| """ | ||
| if C.G == C.G_stand |
There was a problem hiding this comment.
This is almost never going to happen. I'd suggest dropping this case. Also, I think it would go to C.k and not C.n.
But now we have to jump into something deeper that you probably haven't gotten to you. We probably haven't even fixed this completely throughout the library yet either. Some code families don't have a C.G or C.H and they are generated only as needed. Therefore, in functions which work on arbitrary linear codes, we should use G = generator_matrix(C). This will be equivalent to what you wrote in most cases and then generated as needed in the others.
There was a problem hiding this comment.
Ill check if doing this with lazy initialization via getproperty is feasible and won't require big changes in other parts of the code.
| if C.G == C.G_stand | ||
| pivot_inds=[x for x ∈ 1:C.n] | ||
| else | ||
| nonpivot_inds = _non_pivot_cols(C.G, :nsp) |
There was a problem hiding this comment.
So... there's a bug in a separate algorithm and it mainly calls this function. It's probably worth reading this over and seeing if it's actually correct haha. Maybe we should unit test this...
There was a problem hiding this comment.
so _non_pivot_cols currently assumes input is in rref form. I would like to rename it to _rref_non_pivot_cols. Ill add some unit tests
| @@ -91,7 +91,7 @@ export kronecker_product, Hamming_weight, weight, wt, Hamming_distance, distance | |||
| is_valid_bipartition, extract_bipartition, is_Hermitian_self_orthogonal, | |||
| row_supports, row_supports_symplectic, strongly_lower_triangular_reduction, | |||
| residue_polynomial_to_circulant_matrix, group_algebra_element_to_circulant_matrix, | |||
| load_alist | |||
| load_alist, _non_pivot_cols | |||
There was a problem hiding this comment.
Anything starting with a _ is a private function and should not be exported. To use it in the REPL, do CodingTheory._non_pivot_cols.
| # a code with G in standard form | ||
| Gstd = matrix(F, [1 0; 0 1]) | ||
| Cstd = LinearCode(Gstd); | ||
| pivs, info_mat=information_set(Cstd) |
There was a problem hiding this comment.
All seems great. Please insert spaces between equal signs and after commas :)
| Gstd = matrix(F, [1 0; 0 1]) | ||
| Cstd = LinearCode(Gstd); | ||
| pivs, info_mat=information_set(Cstd) | ||
| @test pivs==[1,2] |
There was a problem hiding this comment.
Ironically, your unit test is square and didn't catch the C.n vs C.k issue.
small pr that adds an information set function. Looking for feedback @esabo @benide