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: David Bellot (david.bellot_at_[hidden])
Date: 2010-08-16 17:23:07

Hi Marco,

I was on holiday at the time of your ticket. In fact, I don't work very
often with complex, but you found an interesting bug.
Your patch seems to be pretty reasonable in fact. Can you also create a test
that we can include in the ublas main test.

I'm going to review your patch right now and apply it to the trunk if it
seems correct to me.From what I read from your email, your solution with a
static_cast is reasonable and elegant. I don't want to clutter things with
mpl yet.

As soon as it's done, we can go to your second patch.
Thanks a lot for your help.

Cheers,
David

On Mon, Aug 16, 2010 at 23:06, Marco Guazzone <marco.guazzone_at_[hidden]>wrote:

> 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
> _______________________________________________
> ublas mailing list
> ublas_at_[hidden]
> http://lists.boost.org/mailman/listinfo.cgi/ublas
> Sent to: david.bellot_at_[hidden]
>

-- 
David Bellot, PhD
david.bellot_at_[hidden]
http://david.bellot.free.fr