Re: [ublas] vector/matrix_assign do not work for integral types (bug?)

Subject: Re: [ublas] vector/matrix_assign do not work for integral types (bug?)
From: Marco Guazzone (marco.guazzone_at_[hidden])
Date: 2010-08-16 17:06:30

Hi Nasos,

On Mon, Aug 16, 2010 at 5:15 PM, Nasos Iliopoulos <nasos_i_at_[hidden]> wrote:
> Marco,
> indeed it seems that uBlas was designed without integral types in mind and I
> think that some things can be tweaked to correct for this. Tweaking the
> inequality should not affect anything important, as it only compares with
> the epsilon.
>
> There is only one thing to consider, that in this case uBlas will be
> comparing an integral type with a floating type, introducing an
> inconsistency. I don't know if it is better to use mpl::is_integral to get
> rid of this inconsistency (and hence complicate the code), or go with the
> solution you propose. The way I see it though the <= will not create any
> trouble so I vote going for it atm and just make a note for future
> reference.
>

Maybe I miss something, but I don't see this problem.
Let's analyze the "equals" and "expression_type_check" functions in
"vector_assign.hpp" (similar considerations can be done for
"matrix_assign.hpp"):
--- [code] ---
    template<class E1, class E2, class S>
    BOOST_UBLAS_INLINE
    bool equals (const vector_expression<E1> &e1, const
vector_expression<E2> &e2, S epsilon, S min_norm) {
        return norm_inf (e1 - e2) <= epsilon *
               std::max<S> (std::max<S> (norm_inf (e1), norm_inf
(e2)), min_norm);
    }

    template<class E1, class E2>
    BOOST_UBLAS_INLINE
    bool expression_type_check (const vector_expression<E1> &e1, const
vector_expression<E2> &e2) {
        typedef typename type_traits<typename promote_traits<typename
E1::value_type,
                                     typename
E2::value_type>::promote_type>::real_type real_type;
        return equals (e1, e2, BOOST_UBLAS_TYPE_CHECK_EPSILON,
BOOST_UBLAS_TYPE_CHECK_MIN);
    }
--- [/code] ---

Let e1 and e1 be two vector of ints (ublas::vector<int>)
In "expression_type_check" we have:
* E1::value_type ==> int
* E2::value_type ==> int
* real_type ==> int (type_traits<int>::real_type gives int)
* BOOST_UBLAS_TYPE_CHECK_EPSILON
     ==> type_traits<real_type>::type_sqrt
(std::numeric_limits<real_type>::epsilon ())
     ==> type_traits<int>::type_sqrt (std::numeric_limits<int>::epsilon ())
     ==> int
* BOOST_UBLAS_TYPE_CHECK_MIN
     ==> type_traits<real_type>::type_sqrt (
::boost::is_integral<real_type>::value ? 0 :
((std::numeric_limits<real_type>::min) ()))
     ==> type_traits<int>::type_sqrt (
::boost::is_integral<int>::value ? 0 :
((std::numeric_limits<int>::min) ()))
     ==> int

So when "equals" is called the inner check is done on integral
expressions (note: in this case, norm_inf returns an int type too).

Do you agree?

I have prepared patch for:
* detail/config.hpp
* detail/vector_assign.hpp
* detail/matrix_assign.hpp

I didn't yet posted on TRAC cause of my previously posted patch on
matrix_assign.hpp that waits to be applied (or refused). See:
* http://lists.boost.org/MailArchives/ublas/2010/07/4420.php
* https://svn.boost.org/trac/boost/ticket/4410

What should I do?
a) Post a patch for matrix_assign.hpp based on current ublas trunk
(ignoring that patch)?
b) Post a patch for matrix_assign.hpp, keeping into consideration that
previously submitted patch?

Best,

-- Marco