[vsipl++] vramp test

[Jules Bergmann]

Assem Salama wrote:
 > Everyone,
 >  This test tests the functionality of ramp.

Assem,

This looks good.  I like the flexibility of the tester.  I have a few
comments, but once they are addressed, please check it in.

Since vramp.hpp is only used by vramp.cpp, it would be better to merge
them together as a single .cpp file.  That way the tests are in the
same file as the test driver, which makes it easier to read,
understand, etc.  It also means you don't have to worry about making
vramp.hpp "self-contained", i.e. including all necessary headers and
not relying on vramp.cpp to do that before including it.

I also have a few minor comments below.

				thanks,
				-- Jules


 > ------------------------------------------------------------------------
 >
 > Index: ChangeLog
 > ===================================================================
 > --- ChangeLog	(revision 168042)
 > +++ ChangeLog	(working copy)
 > @@ -1,3 +1,9 @@
 > +2007-04-05  Assem Salama <assem at codesourcery.com>

[1] To be consistent with the other ChangeLog entries, please put a
blank line between the date/author line (above) and the change log
entry (below).

 > +	* tests/parallel/vram.cpp: New file. This file tests the ramp
 > +	  function.

[2] There's a typo in the file name.  Video RAM?  Image processing,
eh?  :)


 > +	* tests/parallel/vramp.hpp: New file. This file contains the
 > +	  do_test structure which holds the tests to be performed.
 > +
 >  2007-04-02  Assem Salama <assem at codesourcery.com>
 >  	* src/vsip/core/expr/generator_block.hpp: Made Choose_peb of
 >  	  Generator_expr_block a Peb_remap_tag. Changed apply function to call
 > Index: tests/parallel/vramp.cpp
 > ===================================================================
 > --- tests/parallel/vramp.cpp	(revision 0)
 > +++ tests/parallel/vramp.cpp	(revision 0)
 > @@ -0,0 +1,115 @@
 > +/* Copyright (c) 2007 by CodeSourcery.  All rights reserved.
 > +
 > +   This file is available for license from CodeSourcery, Inc. under 
the terms
 > +   of a commercial license and under the GPL.  It is not part of the 
VSIPL++
 > +   reference implementation and is not available under the BSD license.
 > +*/
 > +/** @file    tests/parallel/vramp.cpp
 > +    @author  Assem Salama
 > +    @date    2005-04-05

[3] The date is wrong.



 > +
 > +template <int test_num,
 > +          typename ViewT,
 > +	  typename MapT>
 > +int test_vramp(Domain<ViewT::dim> sz)
 > +{
 > +
 > +  const dimension_type                                    dim = 
ViewT::dim;
 > +  typedef typename ViewT::value_type                      T;
 > +  typedef Dense<dim,T,typename Row_major<dim>::type,MapT> block_type;
 > +  typedef typename View_of_dim<dim, T, block_type>::type  view_type;
 > +
 > +  assert(dim == 1); // ramp doesn't work for anything except vectors 
for now

[4] The "for now" implies that eventually we may fix this (i.e. either
Sourcery VSIPL++ doesn't fully implement the spec, or we're planning
to extend the spec).

Since it is a "limitation" of the spec, in the sense that it ony defines
1D ramp, I would say something like:

	"// ramp only works on vectors."

 > +
 > +  // create view
 > +  MapT                map = Create_map<MapT>::exec();
 > +  block_type          block(sz,map);
 > +  view_type           view(block);
 > +
 > +  // assign to a ramp
 > +  do_test<test_num>::exec(view, sz.size());
 > +
 > +#if DEBUG == 1
 > +  std::cout << "View of test "<<test_num<<"\n";
 > +  std::cout << view;
 > +#endif
 > +
 > +  // check results
 > +  {
 > +    Index<dim> idx;
 > +    Length<dim> ext = extent(view);
 > +    T val = T(0);
 > +    for(;valid(ext,idx);next(ext,idx)) {
 > +      assert(do_test<test_num>::check(view,idx,val));

[5] Use test_assert in tests!  assert() gets disabled when -NDEBUG
defined, which we do when compiling the library to go fast.

 > +    }
 > +  }
 > +
 > +  return 0;
 > +}
 > +
 > +
 > +int main(int argc, char **argv)
 > +{
 > +  int size=16;

[6] It is good practice to make 'size' a 'length_type' instead of an
'int'.

 > +
 > +  vsipl vpp(argc,argv);
 > +
 > +  test_vramp<1,Vector<float>, Local_map>     (Domain<1>(size));
 > +  test_vramp<1,Vector<float>, Map<> >        (Domain<1>(size));
 > +  test_vramp<1,Vector<float>, Global_map<1> >(Domain<1>(size));
 > +
 > +  test_vramp<2,Vector<float>, Local_map>     (Domain<1>(size));
 > +  test_vramp<2,Vector<float>, Map<> >        (Domain<1>(size));
 > +  test_vramp<2,Vector<float>, Global_map<1> >(Domain<1>(size));
 > +
 > +  test_vramp<3,Vector<float>, Map<> >        (Domain<1>(size));
 > +  test_vramp<4,Vector<float>, Map<> >        (Domain<1>(size));
 > +  test_vramp<5,Vector<float>, Map<> >        (Domain<1>(size));
 > +  test_vramp<6,Vector<float>, Map<> >        (Domain<1>(size));
 > +
 > +  return 0;
 > +}
 > Index: tests/parallel/vramp.hpp
 > ===================================================================
 > --- tests/parallel/vramp.hpp	(revision 0)
 > +++ tests/parallel/vramp.hpp	(revision 0)
 > @@ -0,0 +1,145 @@
 > +/* Copyright (c) 2007 by CodeSourcery.  All rights reserved.
 > +
 > +   This file is available for license from CodeSourcery, Inc. under 
the terms
 > +   of a commercial license and under the GPL.  It is not part of the 
VSIPL++
 > +   reference implementation and is not available under the BSD license.
 > +*/
 > +/** @file    tests/parallel/vramp.hpp
 > +    @author  Assem Salama
 > +    @date    2005-04-05
 > +    @brief   VSIPL++ Library: Header file for tests of ramp function
 > +*/
 > +
 > +#ifndef TESTS_PARALLEL_VRAMP_HPP
 > +#define TESTS_PARALLEL_VRAMP_HPP
 > +
 > +template <int test_num>
 > +struct do_test;
 > +
 > +
 > +// declare all tests here
 > +
 > +// TEST1: A simple assignment, view = ramp
 > +template <>
 > +struct do_test<1>
 > +{
 > +  template <typename ViewT>
 > +  static void exec(ViewT& view, length_type size)
 > +    { typedef typename ViewT::value_type T; view = 
ramp(T(0),T(1),size); }
 > +
 > +  template <typename ViewT, typename T>
 > +  static int check(ViewT& view, Index<ViewT::dim> idx, T& val)

[7] For efficiency, you should pass structures like 'idx' as a const
references, i.e.  'Index<ViewT::dim> const& idx'.  Performance doesn't
matter too much here in a test, but its a good habit to get into.

 > +    { return (get(view,idx) == val++); }
 > +};
 > +

-- 
Jules Bergmann
CodeSourcery
jules at codesourcery.com
(650) 331-3385 x705