Bug in C++ interface (comparison between rational and double)
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.
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:
- add support for comparison between mpq and double in C interface and
use that in gmpxx.h
- 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 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:
- add support for comparison between mpq and double in C interface and
use that in gmpxx.h
- 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
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 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).
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
participants (4)
-
Abramo Bagnara -
Ed Grimm -
Marc Glisse -
Roberto Bagnara