MIMO Channel Estimation by MLsmd · Pull Request #29 · kit-cel/gnuradio

MLsmd · 2018-07-11T13:20:36Z

Add MIMO channel estimator block in C++.
Add qa test in python for channel estimator block.
Add doxygen documentation.
Add .xml file for use in GRC.

This closes #17.

…ime error if Eigen3 not found.

…d 1x1 and 2x2 MIMO scheme.

…ng the Eigen library.

…buffers.

…() loops for terminated input data.

MLsmd · 2018-07-11T13:23:36Z

Please review and merge the V-BLAST branch #28 first, because #28 is already rebased into this branch #29.

marcusmueller

The uint16 symbol_length overflow issue is the only bug I could find.

Not quite sure about the break issue, might be an oversight.

The PMT performance issues are not important.

marcusmueller · 2018-08-08T14:00:15Z

gr-digital/lib/mimo_channel_estimator_cc_impl.cc

+        // Iterate over tags in buffer.
+        for (unsigned int i = 0; i < tags.size(); ++i) {
+          // Calculate the number of items before the next tag.
+          if (i < tags.size() - 1) {


Why not simply split into a for loop with condition i < tags.size() - 1 and a separate i = tags.size()-1?

That'd enhance readability.

In this case I would have to repeat the code after the if else statement in the for loop for both cases which would worsen the readability again, or am I overseeing something here?

marcusmueller · 2018-08-08T14:02:08Z

gr-digital/lib/mimo_channel_estimator_cc_impl.cc

+            if (symbol_length < d_training_length){
+              /* We consume the whole training sequence in the next buffer and don't consume it now
+               * in order to do the estimation at once. */
+              break;


this is the last iteration of the for loop anyway, and this is the last statement in that iteration. Breaking at this point doesn't do anything, or am I mistaken?

are we perhaps missing the case where symbol length is not smaller than training sequence length and we need to consume now?

The for loop is not finished after the if else statement. If the symbol length is smaller than the training length, we cannot process the received training sequence right now. So we are breaking the loop (we are on the last iteration, but there would still be the copy and tag assignment operations in this iteration to be done) to skip the rest of the code in the for loop.

Wait, I'll have to talk to you personally about this, but this is the "else" belonging to the "if(not last iteration)" clause, and that means this else is only executed on the last iteration, and right after the closing bracket of the "else" clause comes the closing bracket of the "for" clause. I'm clearly missing something here!

marcusmueller · 2018-08-08T14:05:51Z

gr-digital/lib/mimo_channel_estimator_cc_impl.h

+      uint16_t d_N; /*!< Number of receiving antennas. */
+      std::vector<std::vector<gr_complex> > d_training_sequence;
+      /*!< Training matrix: Each subvector/row is sent through one of the M transmit antennas. */
+      uint16_t d_training_length; /*!< Length of the training sequence. */


just out of interest (it's unlikely we find a training sequence longer than 2¹⁶ - 1): Why the uint16_t here?

This has no specific reason. I am changing it to uint32_t.

marcusmueller · 2018-08-08T14:08:59Z

gr-digital/lib/vblast_decoder_cc_impl.cc

+      // Collect all tags of the input buffer in the vector 'tags'.
+      get_tags_in_window(tags, 0, 0, noutput_items);
+
+      uint16_t symbol_length; // Number of items in the current symbol.


are we sure our symbols are limited to 2¹⁶ - 1 items?

marcusmueller · 2018-08-08T14:10:17Z

gr-digital/lib/vblast_decoder_cc_impl.cc

+        if (tags[0].offset - nitems_read(0) > 0){
+          /* There are items in the input buffer, before the first tag arrives,
+           * which belong to the previous symbol. */
+          symbol_length = (tags[0].offset - nitems_read(0))*d_num_inputs;


or rather, limited to (2¹⁶-1)/num_inputs? Sounds a bit of a dangerous thing, considering let's say 8×8 DAB MIMO ;)

I am going to fix these issues in the according branch 'vblast'.

marcusmueller · 2018-08-08T14:12:28Z

gr-digital/lib/vblast_decoder_cc_impl.cc

+            symbol_length = noutput_items - (tags[i].offset - nitems_read(0))*d_num_inputs;
+          }
+          // Check the key of the tag.
+          if (pmt::symbol_to_string(tags[i].key).compare("csi") == 0) {


The whole idea of PMTs having symbols is that a symbol representing the same string is unique. So, instead of converting the symbol to the string and comparing the string you should just have a static pmt_t csi_key here which get initialized once; then you can compare that to the tag key with == directly.

marcusmueller · 2018-08-08T14:12:38Z

gr-digital/lib/vblast_decoder_cc_impl.cc

+            }
+            // Calculate the weighting vector for the next symbol with the received CSI.
+            update_mimo_equalizer();
+          } else if (pmt::symbol_to_string(tags[i].key).compare("snr") == 0 && d_equalizer_type.compare("MMSE") == 0) {


same as above.

The problem with comparing the string really is that PMT is horrific. The symbol_to_string function does a dynamic cast of the pmt_base to a pmt_symbol. That's an awful lot of indirection just to compare. Also, in a lot of cases (I think this might be one), this requires a copy of the std::string contained in that PMT to be made (did I ever mention how bad an idea that design is? Like, have "polymorphic type", but it not be properly polymorphic, and it also just inefficiently wrap std types, for communication within the same process?).

std::string copies are relatively expensive and you try to avoid them in the signal processing part, because they require allocation and deallocation of memory.

marcusmueller · 2018-08-08T14:20:50Z

gr-digital/lib/vblast_encoder_cc_impl.cc

+        gr_complex *out = (gr_complex *) output_items[i];
+        // Write output data to current output port.
+        for (int j = 0; j < noutput_items; ++j) {
+          out[j] = in[j*d_num_outputs + i];


Do we even need this, or would one of the standard deinterleaver blocks work, too?

I think if we swap the i loop with the j loop, it becomes evident that this might be solvable more efficiently with a memcpy

marcusmueller · 2018-08-08T14:23:11Z

gr-digital/python/digital/qa_mimo_channel_estimator_cc.py

+    '''
+    2 tests validating the correct estimation of the channel coefficients with a random channel
+    and 1xN MIMO scheme. '''
+    def test_001_t (self):


This is fine, but for your own sanity in the future: test methods need to start with test_ and can take names like test_heavier_than_duck.

marcusmueller · 2018-08-08T14:23:44Z

gr-digital/python/digital/qa_vblast_decoder_cc.py

+                                                    pmt.make_f32vector(num_inputs, 1e8),
+                                                    pmt.from_long(0))))
+
+        for i in range(0, num_tags):


that's simply range(num_tags) ;)

marcusmueller · 2018-08-08T14:28:23Z

I get merge conflicts. Can you rebase?

… to uint32_t.

MLsmd · 2018-08-08T16:02:23Z

Thanks for the review! As discussed, I opened a new branch with the merged master. I am going to request a new pull and this pull request gets obsolete.

MLsmd added 21 commits June 7, 2018 11:39

VBLAST: Add encoder block and QA test.

9a617cf

VBLAST: Init decoder block.

94b4268

VBLAST: Add precalculated ZF equalizer for 2x2 MIMO scheme.

3c566a9

VBLAST: Add precalculated MMSE equalizer for 2x2 MIMO scheme.

a6db048

VBLAST: Add qa test for 2x2 MMSE case.

01ed484

VBLAST: Add MxM ZF case with Eigen inverse calculation and qa test.

64dba72

VBLAST: Add ZF and MMSE equalizer for MIMO 1x1 scheme (SISO case).

d5f6179

VBLAST: Wrap up qa test for decoder.

d6339ee

VBLAST: Add MxM MMSE case with Eigen pseudo-inverse calculation.

4a127c0

VBLAST: Add loopback test for ZF and MMSE.

896870a

VBLAST: Wrap up code and doxygen docu.

6d1fba3

VBLAST: Add xml files for GRC.

5e0328f

VBLAST: Find package Eigen3 in CMake and replace Eigen code with runt…

a4d1e21

…ime error if Eigen3 not found.

mimo_channel_est: Init channel estimation C++ block with precalculate…

25d6184

…d 1x1 and 2x2 MIMO scheme.

mimo_channel_est: Expand estimation to a general MxN MIMO scheme, usi…

e8995e6

…ng the Eigen library.

mimo_channel_est: Add python qa test for 2x2 scheme.

1b2f71b

mimo_channel_est: Generalize 1x1 and 2x2 MIMO scheme to 1xN and 2xN.

25a6cc3

mimo_channel_est: Wrap up QA test.

482e5e4

mimo_channel_est: Avoid extension of training sequence over multiple …

6b4cf64

…buffers.

mimo_channel_est: Insert head block in qa test to avoid infinite work…

05f4083

…() loops for terminated input data.

mimo_channel_est: Wrap up and docu.

37a3a63

marcusmueller requested changes Aug 8, 2018

View reviewed changes

mimo_channel_est: Change data type of training_sequence from uint16_t…

69ee3e0

… to uint32_t.

Conversation

MLsmd commented Jul 11, 2018

Uh oh!

MLsmd commented Jul 11, 2018

Uh oh!

marcusmueller left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcusmueller Aug 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcusmueller commented Aug 8, 2018

Uh oh!

MLsmd commented Aug 8, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

marcusmueller left a comment •

edited

Loading

marcusmueller Aug 8, 2018 •

edited

Loading