Re: std_min/std_max reference return value is problematic with boost::interval
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.
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.
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.
Michael -- ___________________________________ Michael Stevens Systems Engineering 34128 Kassel, Germany Phone/Fax: +49 561 5218038 Navigation Systems, Estimation and Bayesian Filtering http://www.sf.net/Bayes++ ___________________________________
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
participants (2)
-
Guillaume Melquiond -
Michael Stevens