Number_types: Add Is_zero functor for Gmpq

[afabri]

Summary of Changes

Is this faster than a comparison with zero? And in case we change code where we currently compare with zero, we have to find out if it becomes slower when we apply is_zero() to a double. And what about > 0 ?

Release Management

• Affected package(s):
• Issue(s) solved (if any): fix #0000, fix #0000,...
• Feature/Small Feature (if any):
• Link to compiled documentation (obligatory for small feature) wrong link name to be changed
• License and copyright ownership:

[afabri]

@mglisse Do I also have to check the denominator?

[mglisse]

Why not use the same code as mpq_class, i.e. mpq_sgn(mpq())==0, or mpz_size(mpq_numref(mpq()))==0? Or if you want to use internals, you only need to check if zn->_mp_size is 0.

[mglisse]

Is this faster than a comparison with zero?

Yes! With the changes I suggested, all it does is test if one int is 0, which is definitely faster than anything else.

And in case we change code where we currently compare with zero, we have to find out if it becomes slower when we apply is_zero() to a double.

That would mean that the compiler sucks, because all it does it replace d==0. with a call to an inlinable function that does the same thing.

And what about > 0 ?

We should to it as well, using mpq_sgn(...)>0.

Do I also have to check the denominator?

No.

Side note: for mpq_class with GCC, q==0 already does the right thing, but q==mpq_class(0) obviously doesn't.

[afabri]

In fact with #5333 this becomes somehow irrelevant, or do I miss something?

[afabri]

@mglisse do I have to add Is_zero to both Algebraic_structure_traits and Real_embeddable_traits?

[mglisse]

@mglisse do I have to add Is_zero to both Algebraic_structure_traits and Real_embeddable_traits?

I see in number_utils.h

// We take the Algebraic_structure_traits<>::Is_zero functor by default. If it
//  is not available, we take the Real_embeddable_traits functor

so you probably only need one of them (Algebraic_structure_traits being safer), although it doesn't hurt if you put it in both.

[mglisse]

In fact with #5333 this becomes somehow irrelevant, or do I miss something?

It is still nice to have, for fairer comparisons, or if boost_mp is disabled, or if people use Gmpq directly. I don't think it has been settled yet whether we should use Gmpq or mpq_class/mpq_rational by default for the next release.

[afabri]

It is not so much a question if it hurts or not, but about trying to understand the subtleties. Here two concepts are mentioned but they seem identical. Maybe Michael @mhsaar can explain.

[afabri]

As you mention boost::multiprecision::mpq_rational do also have to add the functors for this type?

[mglisse]

As you mention boost::multiprecision::mpq_rational do also have to add the functors for this type?

I think it already has them?

[afabri]

just found them.

[mglisse]

do also have to add the functors for this type?

One type that is missing Is_positive and Is_negative is Mpzf (they can probably be just x.sign()>0, etc).

[maxGimeno]

Successfully tested in https://cgal.geometryfactory.com/CGAL/testsuite/results-5.3-Ic-158.shtml