Bug in C++ interface (comparison between rational and double)

[Abramo Bagnara]

The comparison between mpz_class and double works fine with +inf and
-inf, while the comparison between mpq_class and double gives a floating
point exception, due to inappropriate mpq_set_d call in eval specialization.

I see two way to fix this:

1) add support for comparison between mpq and double in C interface and
use that in gmpxx.h

2) add infinity (and nan) check in comparison eval specializations in
gmpxx.h

IMHO 1) gives a more complete solution. If you want we're willing to
write the (trivial) patch.

--
Abramo Bagnara

Opera Unica                          Phone: [phone]
Via Borghesi, 16
48014 Castel Bolognese (RA) - Italy
_______________________________________________
gmp-bugs mailing list
gmp-bugs@...
http://gmplib.org/mailman/listinfo/gmp-bugs

[Marc Glisse-3]

On Tue, 27 Jan 2009, Abramo Bagnara wrote:

> The comparison between mpz_class and double works fine with +inf and
> -inf, while the comparison between mpq_class and double gives a floating
> point exception, due to inappropriate mpq_set_d call in eval specialization.
>
> I see two way to fix this:
>
> 1) add support for comparison between mpq and double in C interface and
> use that in gmpxx.h
>
> 2) add infinity (and nan) check in comparison eval specializations in
> gmpxx.h
>
> IMHO 1) gives a more complete solution. If you want we're willing to
> write the (trivial) patch.

Some advantages of solution 2 (I am not saying I am in favor of 2, just
mentionning them for completeness) are that the patch is small
(if(isfinite(d)) current code; else return 0<d;), does not introduce any
new interface, and has absolutely no impact on the code when compiled with
g++ -ffinite-math-only.

It looks like comparison is the only operation where a Inf or NaN double
makes sense in gmpxx (all the others return a gmp type, which can not
represent Inf or NaN) (actually division by Inf could make sense too).

Just curious: what application do you have where it is useful to compare a
gmp type to a non-finite double? (I am not questionning the usefulness of
the feature request but genuinely interested)

--
Marc Glisse

PS: anybody notice on the gmplib webpage that the server migration should
be complete in 9 months?
_______________________________________________
gmp-bugs mailing list
gmp-bugs@...
http://gmplib.org/mailman/listinfo/gmp-bugs

[Paul Zimmermann]

       Marc,

> PS: anybody notice on the gmplib webpage that the server migration should
> be complete in 9 months?

I guess the month should be read in base 1 :-)

Paul
_______________________________________________
gmp-bugs mailing list
gmp-bugs@...
http://gmplib.org/mailman/listinfo/gmp-bugs

[Roberto Bagnara]

Marc Glisse wrote:

> On Tue, 27 Jan 2009, Abramo Bagnara wrote:
>
>> The comparison between mpz_class and double works fine with +inf and
>> -inf, while the comparison between mpq_class and double gives a floating
>> point exception, due to inappropriate mpq_set_d call in eval specialization.
>>
>> I see two way to fix this:
>>
>> 1) add support for comparison between mpq and double in C interface and
>> use that in gmpxx.h
>>
>> 2) add infinity (and nan) check in comparison eval specializations in
>> gmpxx.h
>>
>> IMHO 1) gives a more complete solution. If you want we're willing to
>> write the (trivial) patch.
>
> Some advantages of solution 2 (I am not saying I am in favor of 2, just
> mentionning them for completeness) are that the patch is small
> (if(isfinite(d)) current code; else return 0<d;), does not introduce any
> new interface, and has absolutely no impact on the code when compiled with
> g++ -ffinite-math-only.
>
> It looks like comparison is the only operation where a Inf or NaN double
> makes sense in gmpxx (all the others return a gmp type, which can not
> represent Inf or NaN) (actually division by Inf could make sense too).
>
> Just curious: what application do you have where it is useful to compare a
> gmp type to a non-finite double? (I am not questionning the usefulness of
> the feature request but genuinely interested)

Hi Marc,

in the Parma Polyhedra Library, we provide numerical abstractions that
are parametric with respect to several (extended) number families.
This includes all the native integer and floating point types and
GMP's integers and rationals, possibly augmented with special values
for -inf, +inf and NaN.  On these numerical types, we provide several
operations, including safe (modulo bugs) comparisons.  To implement
those we of course rely on what (we believe) is provided by the
underlying base type.

In this case, we were spoiled by the fact that GMP supports
comparisons between mpz and double via mpz_cmp_d (which can be called
with an infinity).  Without looking at the manual (tsk, tsk, ...)
we assumed the same functionality was provided for mpq.  However,
it seems there is no such a thing as mpq_cmp_d and, as you know,
the implementation of this comparison in gmpxx.h is incorrect.

I don't know if there is any plan to add mpq_cmp_d.  However,
a simple patch to gmpxx.h would fix the problem anyway.
Please let us know if you want us to post the patch.
All the best,

    Roberto

--
Prof. Roberto Bagnara
Computer Science Group
Department of Mathematics, University of Parma, Italy
http://www.cs.unipr.it/~bagnara/
mailto:bagnara@...
_______________________________________________
gmp-bugs mailing list
gmp-bugs@...
http://gmplib.org/mailman/listinfo/gmp-bugs

[Marc Glisse-3]

On Tue, 3 Feb 2009, Roberto Bagnara wrote:

>>> The comparison between mpz_class and double works fine with +inf and
>>> -inf, while the comparison between mpq_class and double gives a floating
>>> point exception, due to inappropriate mpq_set_d call in eval
>>> specialization.

> in the Parma Polyhedra Library, we provide numerical abstractions that
> are parametric with respect to several (extended) number families.
> This includes all the native integer and floating point types and
> GMP's integers and rationals, possibly augmented with special values
> for -inf, +inf and NaN.

Oh, so you extended gmp types with +-inf and NaN, that is interesting. Out
of curiosity, how do you do that?

> On these numerical types, we provide several
> operations, including safe (modulo bugs) comparisons.  To implement
> those we of course rely on what (we believe) is provided by the
> underlying base type.

Ok, so your goal is primarily completeness and correctness for those
extended types, this is a fine goal, I am all for it.

> In this case, we were spoiled by the fact that GMP supports
> comparisons between mpz and double via mpz_cmp_d (which can be called
> with an infinity).

Note that you still need to handle NaN separately (you could consider it
another gmpxx bug that comparison (<,>,<=,>=) with NaN may do something
other than return false).

> Without looking at the manual (tsk, tsk, ...)
> we assumed the same functionality was provided for mpq.  However,
> it seems there is no such a thing as mpq_cmp_d and, as you know,
> the implementation of this comparison in gmpxx.h is incorrect.

You could say that the interaction between gmpxx types and double is only
guaranteed to work in the finite case and the bug is in the missing
documentation of this feature. It feels unsatisfactory to be able to say
z<d (for an infinite d) but have (z-d) crash the program... This would
argue in favor of pushing the appropriate code to a library that augments
the gmpxx types with +-Inf and NaN. Which doesn't mean this won't be done
in gmp...

> I don't know if there is any plan to add mpq_cmp_d.

Looks to me like it would make sense (but as I said in a previous email, I
am not the one who needs convincing).

--
Marc Glisse
_______________________________________________
gmp-bugs mailing list
gmp-bugs@...
http://gmplib.org/mailman/listinfo/gmp-bugs

[Abramo Bagnara]

Marc Glisse ha scritto:
>
> Oh, so you extended gmp types with +-inf and NaN, that is interesting.
> Out of curiosity, how do you do that?

For mpz we overload the _mp_size field using MININT for -inf, MAXINT for
+inf and MININT+1 for NaN.

For mpq we use -N/0 for -inf, +N/0 for +inf and 0/0 for NaN.

For both to check for special values is very fast as it can be done only
testing the sizes and the overwriting of such special values is always
safe inside gmplib.


> Note that you still need to handle NaN separately (you could consider it
> another gmpxx bug that comparison (<,>,<=,>=) with NaN may do something
> other than return false).

Yes you're right, I was missing that.

> You could say that the interaction between gmpxx types and double is
> only guaranteed to work in the finite case and the bug is in the missing
> documentation of this feature. It feels unsatisfactory to be able to say
> z<d (for an infinite d) but have (z-d) crash the program... This would
> argue in favor of pushing the appropriate code to a library that
> augments the gmpxx types with +-Inf and NaN. Which doesn't mean this
> won't be done in gmp...

Here I see three design alternatives:

1) gmplib/gmpxx is designed to return correct results only passing
floating point finite values

2) gmplib/gmpxx is designed to store only finite values, but, when
possible, it handles correctly inf and NaN

3) gmplib/gmpx can store finite/infinite and NaN values and handles them
correctly. mpq is a superset of IEC559 floating point values domain.

Currently we are in a strange situation that we may call 1.5 where some
times infinite values are taken, checked and correctly handled and other
times don't.

IMHO a performance critical application like gmplib should reduce the
handling of special cases in unavoidable paths to the minimum (as
probably they are not needed for many users). We have done the same
thing in the PPL adding a templatic policy argument to our numeric
classes where it's specified if the user code expects special values
treatment or not.

Notwithstanding that, I still think that currently gmplib has a bug (in
code or documentation depending on intended design choices).

--
Abramo Bagnara

Opera Unica                          Phone: [phone]
Via Borghesi, 16
48014 Castel Bolognese (RA) - Italy
_______________________________________________
gmp-bugs mailing list
gmp-bugs@...
http://gmplib.org/mailman/listinfo/gmp-bugs

[Ed Grimm-2]

On Fri, 6 Feb 2009, Abramo Bagnara wrote:

> For mpz we overload the _mp_size field using MININT for -inf, MAXINT
> for +inf and MININT+1 for NaN.
>
> For mpq we use -N/0 for -inf, +N/0 for +inf and 0/0 for NaN.
>
> For both to check for special values is very fast as it can be done
> only testing the sizes and the overwriting of such special values is
> always safe inside gmplib.

<snip>

> IMHO a performance critical application like gmplib should reduce the
> handling of special cases in unavoidable paths to the minimum (as
> probably they are not needed for many users). We have done the same
> thing in the PPL adding a templatic policy argument to our numeric
> classes where it's specified if the user code expects special values
> treatment or not.

Just to add a new opinion to the discussion...

If it is possible to check for all special cases with a single test (for
example, the mpq situation above allows for a single test against the
size of the denominator), it is probably reasonable to test for the
special cases, despite the performance needs.  Under this case, the
normal flow is only distracted for the smallest possible time, for any
number of special cases.

If one needs to make one check per special case (for example, the mpz
situation above), it is probably not reasonable.  If performance is
paramount, one should consider a possible redesign which would enable a
single test to enter all of the special case code.  For example, maybe
have the sentinal value be the _mp_size field holding the MININT value
(since, in two's compliment, negative has one extra value), and use
either _mp_alloc or _mp_d[0] to differentiate between the special cases.

It may be that inf and NaN are something that few applications
encounter.  They are certainly more rare than in IEEE floating point, as
it's much harder to overflow a mpz than it is to overflow a double.
However, any maths that involve dividing relatively big numbers by
relatively little ones can do it, and programming tricks can only delay
that, not eliminate it.  Even mpf math can evaluate to infinite and NaN
values.


If my jumping into the middle of this discussion with my first post
offends anyone, I appologize.  I'm new to gmp, but I'm not new to
performance-critical programming.  A friend of mine recently asked me
for some assistance with some scientific programming he's working on,
and so I ended up here.

Ed
_______________________________________________
gmp-bugs mailing list
gmp-bugs@...
http://gmplib.org/mailman/listinfo/gmp-bugs