[vsipl++] QR Solver

[Stefan Seefeld]

Assem,

I have some high-level comments / suggestions. Sorry for being a bit
behind.


Assem Salama wrote:

> Index: src/vsip/core/cvsip/solver_qr.hpp

Can this file be named src/vsip/core/cvsip/qr.hpp, for consistency
with the other backends ?


> +template <typename T,
> +	  bool     Blocked>
> +class Qrd_impl<T, Blocked, Cvsip_tag>
> +{
> +  typedef vsip::impl::dense_complex_type   complex_type;
> +  typedef Layout<2, col2_type, Stride_unit_dense, complex_type> data_LP;
> +  typedef Fast_block<2, T, data_LP> data_block_type;
> +
> +  // Constructors, copies, assignments, and destructors.
> +public:
> +  Qrd_impl(length_type, length_type, storage_type)
> +    VSIP_THROW((std::bad_alloc));
> +  Qrd_impl(Qrd_impl const&)
> +    VSIP_THROW((std::bad_alloc));

We discussed the use of non-empty exception-specifiers, and came to the conclusion
that such use will likely make the code slower, not faster. I thus think it is best
not to issue 'VSIP_THROW(...)' at all, at least not in non-public parts of the API
that are not covered by the spec.


> +  cvsip::Cvsip_matrix<T>     cvsip_data_;

As Jules suggests, this should become cvsip::View<2, T, true>.

> +  cvsip::Cvsip_qr<T>         cvsip_qr_;

I think it would simplify the code if the cvsip::Cvsip_qr template became
a traits template ('Qrd_traits', or may be even a unified 'Factor_traits').

Then, your Qrd_impl<T, Blocked, Cvsip_tag> class would contain a
Qrd_traits<T>::solver_type * member (or Factor_traits<T>::qr_solver_type *),
and make calls to the static Qrd_traits<T>::decompose(), etc.

This would tidy up the code quite a bit. (See all the other C-VSIPL bindings.)

Thanks,
		Stefan

-- 
Stefan Seefeld
CodeSourcery
stefan at codesourcery.com
(650) 331-3385 x718