Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating an arbitrary precision rational with negative denominator throws exception #27

Open
Schniwarixl opened this issue Oct 9, 2018 · 5 comments

Comments

@Schniwarixl
Copy link

When using a rational with an arbitrary precision integer as given by...

typedef boost::multiprecision::number<boost::multiprecision::cpp_int_backend<0, 0, boost::multiprecision::signed_magnitude, boost::multiprecision::checked>> RationalIntType;

typedef boost::rational Rational;

The following will throw an exception...

Rational test(-2, -4);

This is due to the line

if (den < -(std::numeric_limits::max)()) {
BOOST_THROW_EXCEPTION(bad_rational("bad rational: non-zero singular denominator"));
}

in "template void rational::normalize()"

where std::numeric_limits::max() gives 0. Therefore all negative denominators will throw an exception.

In previous boost version there was no test against max().

@psavouli
Copy link

psavouli commented Feb 12, 2020

This bug was introduced with the fix for #18 released in boost 1.68.

It can be reproduced with the following single line (throws at construction):

boost::multiprecision::cpp_rational rational(1, -2);

As Schniwarixl commented, the problem is the the check using std::numeric_limits<T>::max before ensuring that the denominator is positive. I believe this check is there to handle edge cases like -(-2147483648) which, for example, is not valid for a 32bit signed integer (maximum positive value is 2147483647).

However, std::numeric_limits<T>::max is only meaningful for bounded types. In the case of boost arbitrary precisions types, std::numeric_limits<T>::max will return the default constructed value. For a cpp_int specifically, this is 0.

A solution is to check if the type is bounded or not, using std::numeric_limits<T>::is_bounded before calling std::numeric_limits<T>::max.

@psavouli
Copy link

Opened pull request with the proposed fix plus a unit test: #41.

@mekhontsev
Copy link

The code below throws exception:

boost::rational<boost::multiprecision::cpp_int> r(1, -1)

Solution: the following line in the boost::rational::normalize() :

if (den < -(std::numeric_limits<IntType>::max)()) {

must be replaced with:

if (den == -den) {

@PolarNick239
Copy link

+1, updated from boost 1.64 to 1.77 and encountered the same error.

Workarounded it with

	if (d < 0) {
		n *= -1;
		d *= -1;
	}
        boost::rational<boost::multiprecision::cpp_int> rational(n, d);

@Bolpat
Copy link

Bolpat commented Aug 10, 2023

I found a comment in my company’s code base referencing this issue by link. This seems to be solved (at least in 1.80.0). Why is the issue still open?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants