ledatelescope / bifrost

A stream processing framework for high-throughput applications.
BSD 3-Clause "New" or "Revised" License
64 stars 29 forks source link

Fix transpose to work on single pol data, with test #207

Closed dentalfloss1 closed 11 months ago

dentalfloss1 commented 1 year ago

The diff is actually correct this time! Fixes transpose to work on single polarization data. Includes working with mono wav files in the tutorial. Added a test for these kinds of datasets.

codecov-commenter commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (0388e5f) 65.39% compared to head (fbf27be) 65.39%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #207 +/- ## ======================================= Coverage 65.39% 65.39% ======================================= Files 67 67 Lines 5840 5840 ======================================= Hits 3819 3819 Misses 2021 2021 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

league commented 11 months ago

The successful test speaks for itself! But I'm trying to understand what was broken about it in the first place, and I'm wondering if it can be fixed without special-casing K==1. Was the issue maybe because in_inds_str contained a reference to k when it's mono? Then k wouldn't exist until inside the loop, and in this fix, k is declared without a loop (because the loop would be singleton anyway). Would it be the same to produce something like:

func_str += "enum { K = " + std::to_string(K) + " };\n";
func_str += "int k=0;\n";
func_str +=
    "in_type ivals = in(" + in_inds_str + ");\n"
    "#pragma unroll\n"
    "for( ; k<K; ++k ) {\n"
    "    out(" + out_inds_str + ") = ivals[k];\n"
    "}\n";      

Does that work-alike when K==1 ? Maybe I'm confused about why it wasn't working in the first place?

league commented 11 months ago

I was able to try what I thought might unify the K==1 case with the general case. But it doesn't work! Passing the test is the main thing, so I think this fix can be merged!

When it's passing the tests, here is an example of generated code when K==1:

enum { K = 1 };
int k=0;
out_type ovals = in(k, i0, i1);
out(i0, i1) = ovals;

compared to when K==2:

enum { K = 2 };
in_type ivals = in(i2, i1);
#pragma unroll
for( int k=0; k<K; ++k ) {
    out(k, i1, i2) = ivals[k];
}

Oh, one little simplification incoming in a moment…