Re: [vsipl++] [patch] FIR Filter bank benchmark

[Jules Bergmann <jules@xxxxxxxxxxxxxxxx>]

Don McCoy wrote:

> ------------------------------------------------------------------------
>
> 2006-03-20  Don McCoy  <don@xxxxxxxxxxxxxxxx>
>
> 	* benchmarks/benchmarks.hpp: Updated to reflect new location of 
> 	  test.hpp (see below).  
> 	* benchmarks/firbank.cpp: New file. Implements FIR Filter Bank
> 	  benchmark, one of the MIT/LL PCA Kernel Benchmarks.  Demonstrates
> 	  two algorithms, time-domain convolution and "fast" convolution
> 	  based on Fourier transforms.  Optionally supports using external
> 	  data files where the computed result is compared to the given
> 	  output file.
> 	* src/vsip_csl/test.hpp: Moved from tests/ directory and into the 
> 	  'vsip_csl' namespace.
> 	* src/vsip_csl/output.hpp: Likewise.
> 	* src/vsip_csl/load_view.hpp: Likewise.  Changed Load_view to
> 	  accept only constant filenames.
Don,

This patch looks good.  The only real change I have is you should put 
the output into a global matrix (see below).  Let me know if that makes 
sense.  Once that is changed, please check it in.
				thanks
				-- Jules

> ------------------------------------------------------------------------
>
>
> Index: benchmarks/benchmarks.hpp
> ===================================================================
> RCS file: /home/cvs/Repository/vpp/benchmarks/benchmarks.hpp,v
> retrieving revision 1.1
> diff -c -p -r1.1 benchmarks.hpp
> *** benchmarks/benchmarks.hpp	21 Mar 2006 15:53:09 -0000	1.1
> --- benchmarks/benchmarks.hpp	31 Mar 2006 01:30:32 -0000
> ***************
> *** 18,29 ****
>   // Sourcery VSIPL++ provides certain resources such as system 
>   // timers that are needed for running the benchmarks.
>   
>   #include <vsip/impl/profile.hpp>
> ! #include <../tests/test.hpp>
>   
>   #else
>   
> ! // when linking with non-sourcery versions of the lib, the
>   // definitions below provide a minimal set of these resources.
>   
>   #include <time.h>
> --- 18,33 ----
>   // Sourcery VSIPL++ provides certain resources such as system 
>   // timers that are needed for running the benchmarks.
>   
> + #include <vsip/impl/par-foreach.hpp>
>   #include <vsip/impl/profile.hpp>
> ! #include <vsip_csl/load_view.hpp>
> ! #include <vsip_csl/test.hpp>
> ! 
> ! using namespace vsip_csl;
>   
>   #else
>   
> ! // When linking with non-Sourcery versions of the lib, the
>   // definitions below provide a minimal set of these resources.
>   
>   #include <time.h>
> *************** typedef P_acc_timer<DefaultTime> Acc_tim
> *** 135,141 ****
>   
>   
>   
> - 
>   /// Compare two floating-point values for equality.
>   ///
>   /// Algorithm from:
> --- 139,144 ----
> Index: benchmarks/firbank.cpp
> ===================================================================
> RCS file: benchmarks/firbank.cpp
> diff -N benchmarks/firbank.cpp
> *** /dev/null	1 Jan 1970 00:00:00 -0000
> --- benchmarks/firbank.cpp	31 Mar 2006 01:30:32 -0000
> ***************
> *** 0 ****
> --- 1,466 ----
> + /* Copyright (c) 2006 by CodeSourcery.  All rights reserved. */
> + 
> + /** @file    firbank.cpp
> +     @author  Don McCoy
> +     @date    2006-01-26
> +     @brief   VSIPL++ Library: FIR Filter Bank - MIT Lincoln Labs
> +              Polymorphous Computing Architecture Kernel-Level Benchmarks
Let's jump ahead and use "HPEC" instead of "PCA".

> + */
> + 
> +     
> +     t1.start();
> +     for (index_type l=0; l<loop; ++l)
> +     {
> +       // Perform FIR convolutions
> +       for ( length_type i = 0; i < local_M; ++i )
> +       {
> +         Vector<T> tmp(N, T());
> +         fwd_fft(l_inputs.row(i), tmp);
> +         tmp *= response.row(0);    // assume fft already done on response
> +         inv_fft(tmp, test.row(i)); 
why don't you put the result directly in outputs.local() ?  I see, 
you're using outputs to pass the expected result in.  That's fine.
Instead of declaring 'test' to be a local matrix, can you instead 
declare a global results matrix, use the local view of that matrix here, 
and then check the local portion below?
It is functionally the same as what you're doing here, but it is closer 
to what applications will look like.  After doing a FIRbank, an 
application will probably want to reorganize the data for the next 
operation.  Having the data in a global view makes that possible.
Can make a similar change to the full convolution too?


...



Nice diffs (for the following files)!  You did this manually right?  I 
didn't think CVS handled renaming of files.  Thanks!
> *** tests/test.hpp	2006-03-06 18:15:23.000000000 -0800
> --- src/vsip_csl/test.hpp	2006-03-30 15:51:36.850324000 -0800
> ***************

--
Jules Bergmann
CodeSourcery
jules@xxxxxxxxxxxxxxxx
(650) 331-3385 x705