[vsipl++] [patch] CML backend fir FIR filters

[Don McCoy]

Jules Bergmann wrote:
>   
>> +  Fir_impl(Fir_impl const &fir)
>> +    : base(fir),
>> +      fir_obj_ptr_(NULL),
>> +      filter_state_(fir.filter_state_)
>> +  {
>> +    fir_create(
>> +      &fir_obj_ptr_,
>> +      fir.fir_obj_ptr_->K,
>>     
>
> CML objects are intended to be opaque.
>
> Let's keep this for now, but create an issue (#175) to add a new CML
> attribute function that returns a pointer to the kernel coefficients.
>
> 	float*
> 	cml_fir_attr_kernel_f(
> 	   cml_fir_f* obj);
I should have mentioned this.  It seemed "wrong" to dip beneath the CML
API to get what I wanted, but the above fix will take care of it.  I'll
do this soon.


> What is rt_valid checking exactly?
>
> I think I see.  Does CML FIR not work on fixed size inputs if i % d !=
> 0?  Argh!  That's a mistake on my part with the API design.  I wonder
> how often such cases happen in practice.
>   
Several instances in our test harness were invoking the fir_opt
evaluator instead of ours.  I looked into it and found out this rt_valid
check is the reason.

> Regardless, can you add a comment to that effect?
>
> "CML FIR objects have fixed output size, whereas VSIPL++ FIR objects
> have fixed input size.  If input size is not a multiple of the
> decimation, output size will vary from frame to frame."
>
>
>   
I'll add the comment and leave it that way for now.  I'm not 100% sure
that CML won't do what we want, but I did try it.  It leads to this
error because it ends up exceeding the length of the output vector:


i = 1024, os*d = 1026
fir: ../vpp/src/vsip/core/subblock.hpp:293:
vsip::impl::Subset_block<Block>::Subset_block(const
vsip::Domain<vsip::impl::Subset_block<Block>::dim>&, Block&) [with Blo\
ck = vsip::Dense<1u, float, vsip::tuple<0u, 1u, 2u>, vsip::Local_map>]:
Assertion `dom_[d].size() == 0 || dom_[d].impl_last() < blk_->size(dim,
d)' failed.
/bin/sh: line 1: 14399 Aborted                 ./tests/fir


The chained FIR filter test is written as

  vsip::length_type got1a = 0;
  for (vsip::length_type i = 0; i < 2 * M; ++i) // chained
  {
    got1a += fir1a(
      input(vsip::Domain<1>(i * N, 1, N)),
      output1(vsip::Domain<1>(got1a, 1, (N + D - 1) / D)));
  }

The value got1a is incrementing by a fixed interval each time, because
the operator() function (which calls cml_fir_apply_f()) always returns
the same size. 

So it is as you suggested -- that CML essentially expects to process
even multiples of the output size each time.  It would take a change in
the apply function to make this work correctly -- such that 'apply'
returns the correct number of output values calculated on each
iteration.  Yet this might not be all that simple.

Just an historical note: I copied this implementation, including the
rt_valid check, from the IPP implementation.  Perhaps they had a good
reason for having that restriction as well...


-- 
Don McCoy
don (at) CodeSourcery
(888) 776-0262 / (650) 331-3385, x712