Skip to content

Fix nrLDPCEncode(algo="sionna") for valid BG2/K=640 inputs - #80

Open
ThereWasAYang wants to merge 4 commits into
catkira:masterfrom
ThereWasAYang:master
Open

ThereWasAYang wants to merge 4 commits into
catkira:masterfrom
ThereWasAYang:master

Conversation

@ThereWasAYang

@ThereWasAYang ThereWasAYang commented Apr 14, 2026

Copy link
Copy Markdown

Related Issue

Fixes #78

Summary

This PR fixes a bug in nrLDPCEncode(..., algo="sionna") for valid BG2 low-rate inputs such as:

  • tbs = 552
  • R = 120 / 1024
  • BGN = 2
  • K = 640
  • Zc = 64

Previously, this case failed with a matrix dimension mismatch during encoding.

Root cause

In the current implementation, k_b is used for two different purposes:

  1. selecting the lifting-size set (i_ls)
  2. selecting the number of systematic base-graph columns passed into _gen_submat(...)

For BG2 and K=640, the code correctly selects:

  • k_b = 9

but this value is only intended for lifting-size selection.

When constructing the encoding submatrices, _gen_submat(...) should still use the base-graph systematic width:

  • nsys = 10 for BG2

Using k_b=9 here causes the base graph to be sliced incorrectly, which leads to a dimension mismatch later in _encode(...).

Fix

Keep k_b for lifting-size selection, but use nsys when calling _gen_submat(...).

Before

pcm_a, pcm_b_inv, pcm_c1, pcm_c2 = _gen_submat(bm, k_b, Zc, bgn)

After

pcm_a, pcm_b_inv, pcm_c1, pcm_c2 = _gen_submat(bm, nsys, Zc, bgn)

I also removed the unused pcm = _lift_basegraph(bm, Zc) line from this branch.

Regression test

This PR adds a regression test covering a valid BG2/K=640 case:

  • tbs = 552
  • R = 120 / 1024

The test verifies that nrLDPCEncode(..., algo="sionna") no longer crashes and returns an output with the expected shape.

Observed error before this fix

For the valid input above, the encoder failed with:

ValueError: matmul: dimension mismatch with signature (n,k=576),(k=640,1?)->(n,1?)

Why this is correct

For BG2, the reduced k_b values (10/9/8/6) are part of the lifting-size selection procedure in 38.212. They do not change the actual number of systematic base-graph columns, which remains fixed by the base graph itself.

So:

  • k_b should affect i_ls selection
  • nsys should affect _gen_submat(...) slicing

ThereWasAYang and others added 4 commits April 14, 2026 23:27
…sition

PDSCHDMRSSyms() appended an extra DMRS at symbol 7 for any 8- or 9-symbol
PDSCH allocation, regardless of DMRSAdditionalPosition. When
DMRSAdditionalPosition == 0 the type A single-symbol DMRS must contain only
the front-loaded symbol l0, but the function returned two symbols.

All other duration branches (10/11, 12, 13/14) already gate the additional
DMRS on add_pos; the [8, 9] branch was missing that check. Add the
`if add_pos >= 1` guard so the result matches 3GPP TS 38.211
Table 7.4.1.1.2-3.

This also corrects the downstream APIs that rely on PDSCHDMRSSyms:
nrPDSCHDMRSIndices, nrPDSCHDMRS, nrPDSCHIndices and nrPDSCHPTRS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nrLDPCEncode(algo="sionna") fails for valid BG2 K=640 input because _gen_submat uses k_b instead of nsys

1 participant