Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix incorrect results in string-returning methods of Rationals
Fixes RT#126016: https://rt.perl.org/Ticket/Display.html?id=126016
The issues observed in the ticket result from floating point math noise when
handling the fractional part after splitting the Rational into whole part and
fractional parts. That fractional part remains a Rat, and when further
operations on it coerce it into a Num, the f.p. math noise results in it being
a whole number, but still treated under the assumption of being less than 1.

Fix by checking for that case and adjusting fractional/whole parts accordingly.
Also, fix Rat.perl.EVAL roundtrip for Rats that end up not being equal to the
original Rat due to f.p. math noise by outputting those in <nu/de> notation.

Performance impact of this commit: Rational.Str about 40% faster,
Rational.base about 30% faster, Rat.perl 5% slower
  • Loading branch information
zoffixznet committed Nov 18, 2016
1 parent b4592c0 commit b5aa3c5
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 8 deletions.
9 changes: 6 additions & 3 deletions src/core/Rat.pm
Expand Up @@ -14,9 +14,12 @@ my class Rat is Cool does Rational[Int, Int] {
$d = $d div 2 while $d %% 2;
self.REDUCE-ME;
}
$d == 1
?? self.base(10,*)
!! '<' ~ $!numerator ~ '/' ~ $!denominator ~ '>'
if $d == 1 and (my $b := self.base(10,*)).Numeric === self {
$b;
}
else {
'<' ~ $!numerator ~ '/' ~ $!denominator ~ '>'
}
}
}
}
Expand Down
19 changes: 14 additions & 5 deletions src/core/Rational.pm
Expand Up @@ -72,7 +72,13 @@ my role Rational[::NuT, ::DeT] does Real {
if nqp::istype($!numerator,Int) {
my $whole = self.abs.floor;
my $fract = self.abs - $whole;
my $result = ($!numerator < 0 ?? '-' !! '') ~ $whole;

# fight floating point noise issues RT#126016
if $fract.Num == 1e0 { $whole++; $fract = 0 }

my $result = nqp::if(
nqp::islt_I($!numerator, 0), '-', ''
) ~ $whole;

if $fract {
my $precision = $!denominator < 100_000
Expand Down Expand Up @@ -121,11 +127,14 @@ my role Rational[::NuT, ::DeT] does Real {
$prec = ($!denominator < $base**6 ?? 6 !! $!denominator.log($base).ceiling + 1);
}

my $sign = $!numerator < 0 ?? '-' !! '';
my $whole = self.abs.floor;
my $fract = self.abs - $whole;
my $result = $sign ~ $whole.base($base);
my $sign = nqp::if( nqp::islt_I($!numerator, 0), '-', '' );
my $whole = self.abs.floor;
my $fract = self.abs - $whole;

# fight floating point noise issues RT#126016
if $fract.Num == 1e0 { $whole++; $fract = 0 }

my $result = $sign ~ $whole.base($base);
my @conversion := <0 1 2 3 4 5 6 7 8 9
A B C D E F G H I J
K L M N O P Q R S T
Expand Down

0 comments on commit b5aa3c5

Please sign in to comment.