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

Initializing cpp_rational with 1 / X yields wrong value #540

Closed
Xirema opened this issue Mar 10, 2023 · 2 comments · Fixed by #542
Closed

Initializing cpp_rational with 1 / X yields wrong value #540

Xirema opened this issue Mar 10, 2023 · 2 comments · Fixed by #542

Comments

@Xirema
Copy link

Xirema commented Mar 10, 2023

It appears this has been a problem since 1.78.0, and is still affecting the current version of multiprecision.

Attempting to construct a cpp_rational with a rational value of 1 divided by a cpp_int value of anything else results in the final value being 1, instead of the correct value.

This can be worked-around by ensuring that the divided-by value is explicitly cast to a cpp_rational type before performing the division.

Curiously, this only occurs if the Numerator is 1; if the Numerator is any other value, the rational is correctly initialized.

Godbolt Link demonstrating that the behavior was different (correct) in 1.77.0 and persists up through 1.81.0

Sample Code to reproduce the issue:

#include<boost/multiprecision/cpp_int.hpp>
#include<iostream>

int main() {
    using Integer = boost::multiprecision::cpp_int;
    using Rational = boost::multiprecision::cpp_rational;

    Integer one = 1;
    Integer tenThousand = 10000;
    Integer threeFourFiveSix = 3456;
    Rational oneInTenThousand = Rational(one) / tenThousand;
    Rational oneInThreeFourFiveSix = Rational(one) / threeFourFiveSix;

    std::cout << one << "\n" << tenThousand << "\n" << oneInTenThousand << "\n";
    std::cout << oneInThreeFourFiveSix << "\n";

    Integer two = 2;
    Rational twoInTenThousand = Rational(two) / tenThousand;
    std::cout << twoInTenThousand << "\n";

    Rational oneInTenThousandCorrected = Rational(one) / Rational(tenThousand);
    std::cout << oneInTenThousandCorrected << "\n";
}

Output in 1.77.0:

1
10000
1/10000
1/3456
1/5000
1/10000

Output in 1.81.0:

1
10000
1
1
1/5000
1/10000
@mborland
Copy link
Member

In 1.78.0 the rational adapter was changed here. This looks to be similar to #541 where implicit conversions were dropped by the change. @jzmaddock would know best.

@jzmaddock
Copy link
Collaborator

Testing a fix now.

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

Successfully merging a pull request may close this issue.

3 participants