[boost] Re: std_min/std_max reference return value is problematic with boost::interval

From: Guillaume Melquiond (guillaume.melquiond_at_[hidden])
Date: 2004-03-09 07:47:49

Le mar 09/03/2004 à 10:41, Michael Stevens a écrit :
> Eric, Guillaume, (sorry about the previously half send while I was composing)
> 
> Thanks for the info, I think the solution is partly simple, partly complex.

> > Eric: What strikes me as odd with this overload, though, is that max(a,b) in 
> > this case will return something that is not equal to either a or b. That 
> > seems counter-intuitive. Is this right?
> 
> Firstly it is now clear to me (as stated in the documentation) that 
> max(interval_a,interval_b) is not std::max. The difference in semantics is 
> however only represented by the difference in return value.

And by the namespace, and by the documentation, and by the code :-).

> > Me: Why gcc thinks it needs a temporary here is beyond me.
> I was confused here. I thought std_max would be choosing the std::max overload 
> in this case and not the boost::numeric::interval::max overload. The later is 
> however more specialised.
> 
> > Guillaume: Consequently, since std_max has been defined so that it
> > returns a reference, it can't be used when one argument is an interval
> > and GCC is right to complain.
> GCC is correctly complain. The new std_max/ std_min should ONLY be used as a 
> where we specifically require std::max/min. Therefore the committed changes 
> to interval (particular utility.hpp) are incorrect. 

Not sure what you mean here. The names of std_max and std_min are bad,
these functions should in fact be named dependent_max and dependent_min
(this name was suggested on the mailing-list, but the changes to new
names have been on hold till things settle down).

Consequently, the changes to utility.hpp are right imho. The interval
functions min and max use versions of min and max dependent on the base
type.

> > Eric: The fix is to detect whether the unqualified call to max returns an 
> > lvalue or not and define the return type of std_max appropriately. Easy 
> > enough.
> Given that std::max/min in the standard are defined to return a reference this 
> fix would seem unnecessary/wrong. We don't want to use boost::std_max in this 
> case.

> > However, in the particular case of uBLAS, the problem lies elsewhere
> > imho. At lines 620-621, 688-689, 753-754 of ublas/traits.hpp in the
> > equals function, uBLAS should not be using the norm_inf function. Indeed
> > norm_inf relies on abs(interval); and what is really needed is a
> > norm_inf_for_equals function that would rely on norm(interval). The same
> > problem will also appear later with complex intervals.
> Thanks for following this up and the info. The maths needs checking here. Lets 
> see what Joerg thinks. For a Boost 1.31.x release we should change the 
> mentioned lines in traits.hpp to use boost::numeric::interval::min/max so we 
> at least do not have a regression.

Not sure if it actually is a regression anyway since the original code
seems to me to have been wrong from the start. Since uBLAS equality
relies on a fuzz factor (the epsilon macro), it should not throw when
the equality is not certain (and unfortunately, the order relation on
intervals will make it uncertain). It's why I was suggesting to switch
from abs to norm since interval computations are not necessary in the
equality test. I'm not sure my explanations are clear.

Regards,

Guillaume