Re: [Discuss-gnuradio] volk kernels and num_bytes vs num_points

[Tom Rondeau]

On Mon, Jul 2, 2012 at 6:34 PM, Josh Blum <address@hidden> wrote:
> Hey VOLK enthusiasts,
>
> It looks like in just a few kernels we use num_bytes as the length
> parameter rather than num_points. I think this may have been a source of
> confusion and perhaps a few test failures.

Yes, this has been bugging me, too. Thanks for tackling it.

> See, we pass random number of points into these kernels for test. But,
> when random numbers of points were interpreted as bytes, you get caught
> slicing points/items down the middle... I found this out the hard way
> when doing some alignment work.
>
> I have pushed a branch that fixes 12 kernels that do this. I hope we can
> merge this into the "next" branch. Since, I suppose its an API change.
> Or we call it a fix and merge it into maint/master. Anyways...
>
> It looks like the only code affected is in gr-filter. I suggest Tom take
> a second look at the usage of volk_32fc_x2_dot_prod_32fc. Whether or not
> we take the changeset, it might be good to verify that the following
> code isnt being bitten by this usage:
>
>> ./gr-filter/lib/fir_filter.cc:        volk_32fc_x2_dot_prod_32fc_a(d_output, 
>> ar,
>> ./gr-filter/lib/fir_filter_with_buffer.cc:    
>> volk_32fc_x2_dot_prod_32fc_a(d_output, ar,
>> ./gr-filter/lib/fir_filter_with_buffer.cc:    
>> volk_32fc_x2_dot_prod_32fc_a(d_output, ar,
>
>
> And here is the commit for reference:
> http://gnuradio.org/cgit/jblum.git/commit/?h=volk_next_use_num_points
>
> -josh

It'd be great to get this in, but yes, it does constitute and "API
change" so it might have to wait. We're steadily pushing on with 3.7,
though, so putting it in next shouldn't keep it from being used for
too long. On the other hand, as you say, it's used in gr-filter. We're
trying to remove an bugs there, and perhaps the reason for some bugs
is due to this. If we only apply this to next, we'll have to ask
anyone having problems to try it out there and see.

So that's my thought right now for taking this patch based on our
policy of what goes in master. I'll think some on this to see if it's
critical/necessary and if we should make an exception.

Thanks,
Tom