fixed tracking of column swaps and also adjusted column swaps when ma…

[daniel-x]

…king an FEC matrix systematic

Description

bugfix pull request to fix one bug.

Fixed Bug

the problem that shall be fixed is a wrong tracking of the column swaps behind a call to sionna.fec.utils.pcm2gm, more specifically in sionna.fec.utils.make_systematic . there, parts of a matrix get swapped correctly, but the tracked column swaps do not reflect the swaps actually performed. the swap-tracking is wrong. this is easy to see when using sionna.fec.utils.pcm2gm and telling it to check its own result for correctness. it fails for certain cases. one such case is in the below file, which is a minimum runnable example that demonstrates the bug:
https://github.com/daniel-x/sionna_daniel-x/blob/fix_column_swap_tracking_demo/bug_demo_pcm2gm_wrong_column_swap_tracking.py
(the bug demo is intentionally NOT part of the pull request, because it shall not end up in the main branch)

Solution

the current version with the bug does a blockwise swap of matrix regions. it is not straightforward to see how to reflect this with individual column swaps (though it is possible). this was likely the reason for the bug.
the solution replaces the blockwise swap of matrix regions by individual column swaps and tracks all swaps correctly. this has 3 effects:

1. the performed column swaps get tracked correctly and the result is valid for more input samples
2. less memory usage, because there is no more need to keep temporary copies of matrix regions in memory
3. the exact result before and after this bugfix is different, because the column rearrangement strategy is changed. however, the result is still valid, because as long as the column reordering is tracked correctly and the matrix regions as a whole are swapped, the result is valid.

Point 3. might break tests, although the result is still valid. In this case, the tests need to be examined carefully and adjusted if the results are still correct.

Necessity for the change

The code currently fails to calculate the correct generator matrices for valid check matrices of certain kinds. This makes it hard to test other error correction codes and compare them using sionna, making interoperability a burden, hence users might switch to other tools or libraries or cause confusion. To make sionna more usable, the change should be pulled.

Checklist

[+] Detailed description
[-] Added references to issues and discussions
[-] Added / modified documentation as needed
[-] Added / modified unit tests as needed
[ ] Passes all tests
[+] Lint the code
[+] Performed a self review
[ ] Ensure you Signed-off the commits. Required to accept contributions!
[-] Co-authored with someone? Add Co-authored-by: user@domain and ensure they signed off their commits too.

[SebastianCa]

Hi,
Thank you for this PR and the detailed description. We review the code in the next days and provide detailed feedback.

[daniel-x]

I've added a fix on the same branch for another bug (make_systematic_wrong_1_search) for the same function make_systematic. The two bugs have no relation besides that they are in the same function. They are so nearby in the code that it makes sense to review them together, so I committed them to the same branch and thus to the same pull request.

I've added another MRE for reproducing the second bug:
https://github.com/daniel-x/sionna_daniel-x/blob/fix_column_swap_tracking_demo/bug_demo_make_systematic_wrong_1_search.py
(the MRE is not part of the pull request, thus not committed to the pull request's branch)

[SebastianCa]

The proposed changes are ok and will be merged in the next release of Sionna.

Thank you again for this PR.

[gmarcusm]

Thanks for your contribution. In order to merge it, please add a Sign-off to your commits.

[SebastianCa]

Hi @daniel-x, it seems like something went wrong with the PR and you deleted the branch.

We would just need a signed-off commit to be able to merge with the main branch of Sionna.