Fix cyclic prefix length of first symbol for 5g NR PUSCH

[danielschaeufele]

Description

According to TS 138.211, section 5.3.1, the length of the cyclic prefix is extended by 16κ for the first symbol of each half subframe.

An attempt to implement this was done in CarrierConfig.cyclic_prefix_length, but the code checks for the slot number instead of the symbol number.

See also #425.

• Fixes a bug?

In order to support a different cyclic prefix lengths for the first symbol of each "block" (a half subframe in 5G NR and a slot in LTE), the parameters cyclic_prefix_length_first_symbol and symbols_per_block were added to OFDMModulator/OFDMDemodulator. The algorithm of OFDMModulator/OFDMDemodulator was modified to reshape the data into blocks (while adding zero padding beforehand if the number of OFDM symbols is not an integer multiple of symbols_per_block). Then the additional cyclic prefix is added/removed from the beginning of each block. Afterwards the data is reshaped into the proper shape and the zero padding is removed (if necessary).

The property cyclic_prefix_length_first_symbol was added to CarrierConfig and PUSCHTransmitter/PUSCHReceiver were modified to pass this parameter to OFDMModulator/OFDMDemodulator.

• Adds a new feature?

No

• Introduces API changes?

Optional parameters have been added to OFDMModulator/OFDMDemodulator. CarrierConfig got a new property cyclic_prefix_length_first_symbol and the return value of cyclic_prefix_length might change.

5G PUSCH data sets generated with the old version can't be loaded with the new version. Unfortunately, I don't see a way to fix this bug while being backwards compatible.

• Other contributions

Fixed some spelling errors.

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!

[danielschaeufele]

I just noticed an issue with my code. The assumption that the first symbol in a slot has a longer cyclic prefix is not always valid. For example with SCS = 60kHz only every second slot will have a longer cyclic prefix for its first symbol. The solution is pretty straightforward (check for the slot number in CarrierConfig.cyclic_prefix_length_first_symbol), but I'm not 100% sure if I understand how the PUSCHReceiver is supposed to be used. Am I correct in assuming that to decode 10 consecutive slots of a recording, 10 different PUSCHReceiver objects with different values for slot_number have to be created? Or is there an alternative way that also needs to be supported?

I'm also not super happy about all the additional properties my changes introduced. Alternatively I could also modify CarrierConfig.cyclic_prefix_length to return a list with one value for each symbol. Then the OFDMModulator/OFDMDemodulator would need to contain a loop which deals with each symbol individually. Unfortunately things like these are rather annoying to implement in Tensorflow. Which variant do you like better?

[jhoydis]

Hi @danielschaeufele,

Yes, in the current implementation, the slot number cannot be changed on the fly. That's one of things we might change in the future. However, this requires some deeper level of integration and cannot be added as a simple functionality on top of the existing code.

Concerning your pull request, it is great that you managed to add these changes. However, we currently cannot allocate the necessary time to review it properly. Hope for your understanding. Nevertheless, your fork might still be useful for other people although it is not integrated yet.

[danielschaeufele]

FYI: I just did a complete rewrite of my implementation. Instead of passing the length of long and normal cyclic prefixes and the periodicity of the long cyclic prefix, I pass the cyclic prefix length as a vector now. Instead of reshaping multiple times to add/remove the cyclic prefix I use tf.gather now, which is much cleaner and (to my surprise) faster than my first implementation.

There is one remaining issue: RessourceGrid takes only a single value for its cyclic prefix length parameter. As far as I can see, this is only used in the channel simulation code, so I just passed the length of a normal cyclic prefix, which will give the same (but slightly wrong) result as before my change. Properly fixing this would require a rewrite of the channel simulation code.

[jhoydis]

Hi @danielschaeufele,

The next Sionna release will comprise updated OFDMModulator and OFDMDemodulator classes which look very similar to what you have done (the main difference is that we avoid the costly gather operation if all CPs are the same). Once the release is there, it would be great if you could update your PR to use these new classes. The interface is the same as what you have used, i.e., providing the CP lengths as vector.

I still need to review the rest and think about the implications for the resource grid. I think that it is also used to compute the noise power in many OFDM-based link-level simulations. Maybe we could also use a vector for the CP lengths here.

[danielschaeufele]

Hi @jhoydis,

I updated my code to work with the new release.

The problem with RessourceGrid is still open, but this can also be fixed at a later time, because now the channel simulation code gives the same result as before.

When this PR is merged, I can also prepare a PR for #494.