Skip to content

Commit

Permalink
let's try 'better' approximating MRI's BigDecimal precision (/ and *)
Browse files Browse the repository at this point in the history 
esp. with division/multiplication - some parts now match C source closer
however there's a fundamental difference in terms of MRI keeping the
precision of a constructed number, while Java's BigDecimal doesn't ...

thus we will tend to end up with different precisions based on input.
this needs to be cared out carefully as going 'too far', as some of the
ruby specs never finish computing (division atm) ...

closes jruby#3846, jruby#4200 (we have tests guarding against regression)
  • Loading branch information
@kares
kares committed Apr 15, 2018
1 parent b1d0cc0 commit 2b06a12
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 45 deletions.
93 changes: 54 additions & 39 deletions core/src/main/java/org/jruby/ext/bigdecimal/RubyBigDecimal.java
View file
Expand Up @@ -139,6 +139,8 @@ public RubyBigDecimal allocate(Ruby runtime, RubyClass klass) {
// #elif SIZEOF_BDIGITS >= 4
// # define RMPD_COMPONENT_FIGURES 9
private static final short RMPD_COMPONENT_FIGURES = 9;
// rmpd_component_figures(void) { return RMPD_COMPONENT_FIGURES; }
// #define VpBaseFig() rmpd_component_figures()
private static final short BASE_FIG = RMPD_COMPONENT_FIGURES;

// Static constants
Expand Down Expand Up @@ -279,7 +281,7 @@ public IRubyObject _dump(ThreadContext context, IRubyObject unused) {
@JRubyMethod(meta = true)
public static RubyBigDecimal _load(ThreadContext context, IRubyObject recv, IRubyObject from) {
String precisionAndValue = from.convertToString().asJavaString();
String value = precisionAndValue.substring(precisionAndValue.indexOf(":")+1);
String value = precisionAndValue.substring(precisionAndValue.indexOf(':') + 1);
return newInstance(context, recv, RubyString.newString(context.runtime, value));
}

Expand Down Expand Up @@ -452,7 +454,18 @@ private static BigDecimal toBigDecimal(final RubyInteger value) {
return new BigDecimal(value.getBigIntegerValue());
}

private static RubyBigDecimal getVpValue19(ThreadContext context, IRubyObject value, boolean must) {
private static RubyBigDecimal getVpRubyObjectWithPrecInner(ThreadContext context, RubyRational value, RoundingMode mode) {
BigDecimal numerator = toBigDecimal(value.getNumerator());
BigDecimal denominator = toBigDecimal(value.getDenominator());

int len = numerator.precision() + denominator.precision(); // 0
int pow = len / 4; // 0
MathContext mathContext = new MathContext((pow + 1) * 4, mode);

return new RubyBigDecimal(context.runtime, numerator.divide(denominator, mathContext));
}

private RubyBigDecimal getVpValueWithPrec(ThreadContext context, IRubyObject value, boolean must) {
if (value instanceof RubyFloat) {
double doubleValue = ((RubyFloat) value).getDoubleValue();

Expand All @@ -463,10 +476,11 @@ private static RubyBigDecimal getVpValue19(ThreadContext context, IRubyObject va
throw context.runtime.newFloatDomainError("NaN");
}

return new RubyBigDecimal(context.runtime, BigDecimal.valueOf(doubleValue));
MathContext mathContext = new MathContext(RubyFloat.DIG + 1, getRoundingMode(context.runtime));
return new RubyBigDecimal(context.runtime, new BigDecimal(doubleValue, mathContext));
}
if (value instanceof RubyRational) {
return getVpRubyObjectWithPrecInner(context, (RubyRational) value, getRoundingMode(context.runtime));
return div2Impl(context, ((RubyRational) value).getNumerator(), ((RubyRational) value).getDenominator(), this.value.precision() * BASE_FIG);
}

return getVpValue(context, value, must);
Expand All @@ -488,17 +502,6 @@ private static RubyBigDecimal getVpValue(ThreadContext context, IRubyObject valu
return cannotBeCoerced(context, value, must);
}

private static RubyBigDecimal getVpRubyObjectWithPrecInner(ThreadContext context, RubyRational value, RoundingMode mode) {
BigDecimal numerator = toBigDecimal(value.getNumerator());
BigDecimal denominator = toBigDecimal(value.getDenominator());

int len = numerator.precision() + denominator.precision(); // 0
int pow = len / 4; // 0
MathContext mathContext = new MathContext((pow + 1) * 4, mode);

return new RubyBigDecimal(context.runtime, numerator.divide(denominator, mathContext));
}

@JRubyMethod(meta = true)
public static IRubyObject induced_from(ThreadContext context, IRubyObject recv, IRubyObject arg) {
return getVpValue(context, arg, true);
Expand Down Expand Up @@ -816,20 +819,19 @@ public IRubyObject initialize_copy(IRubyObject original) {
throw getRuntime().newTypeError("wrong argument class");
}

RubyBigDecimal origRbd = (RubyBigDecimal)original;
RubyBigDecimal orig = (RubyBigDecimal) original;

this.isNaN = origRbd.isNaN;
this.infinitySign = origRbd.infinitySign;
this.zeroSign = origRbd.zeroSign;
this.value = origRbd.value;
this.isNaN = orig.isNaN;
this.infinitySign = orig.infinitySign;
this.zeroSign = orig.zeroSign;
this.value = orig.value;

return this;
}

@JRubyMethod(name = {"%", "modulo"}, required = 1)
public IRubyObject op_mod(ThreadContext context, IRubyObject other) {
// TODO: full-precision divmod is 1000x slower than MRI!
RubyBigDecimal val = getVpValue19(context, other, false);
RubyBigDecimal val = getVpValueWithPrec(context, other, false);

if (val == null) return callCoerced(context, sites(context).op_mod, other, true);
if (isNaN() || val.isNaN() || isInfinity() && val.isInfinity()) return newNaN(context.runtime);
Expand All @@ -853,7 +855,7 @@ public IRubyObject op_mod19(ThreadContext context, IRubyObject arg) {
@Override
@JRubyMethod(name = "remainder", required = 1)
public IRubyObject remainder(ThreadContext context, IRubyObject arg) {
return remainderInternal(context, getVpValue19(context, arg, false), arg);
return remainderInternal(context, getVpValueWithPrec(context, arg, false), arg);
}

@Deprecated
Expand All @@ -862,7 +864,6 @@ public IRubyObject remainder19(ThreadContext context, IRubyObject arg) {
}

private IRubyObject remainderInternal(ThreadContext context, RubyBigDecimal val, IRubyObject arg) {
// TODO: full-precision remainder is 1000x slower than MRI!
if (isInfinity() || isNaN()) return newNaN(context.runtime);
if (val == null) return callCoerced(context, sites(context).remainder, arg, true);
if (val.isInfinity() || val.isNaN() || val.isZero()) return newNaN(context.runtime);
Expand All @@ -873,7 +874,7 @@ private IRubyObject remainderInternal(ThreadContext context, RubyBigDecimal val,

@JRubyMethod(name = "*", required = 1)
public IRubyObject op_mul(ThreadContext context, IRubyObject arg) {
RubyBigDecimal val = getVpValue19(context, arg, false);
RubyBigDecimal val = getVpValueWithPrec(context, arg, false);
if (val == null) return callCoerced(context, sites(context).op_times, arg, true);
return multImpl(context.runtime, val);
}
Expand All @@ -888,7 +889,7 @@ public IRubyObject mult2(ThreadContext context, IRubyObject b, IRubyObject n) {
final int mx = getPrecisionInt(context.runtime, n);
if (mx == 0) return op_mul(context, b);

RubyBigDecimal val = getVpValue19(context, b, false);
RubyBigDecimal val = getVpValueWithPrec(context, b, false);
if (val == null) { // TODO: what about n arg?
return callCoerced(context, sites(context).op_times, b, true);
}
Expand Down Expand Up @@ -1036,7 +1037,7 @@ private BigDecimal powNegative(final int times) {

@JRubyMethod(name = "+")
public IRubyObject op_plus(ThreadContext context, IRubyObject b) {
return addInternal(context, getVpValue19(context, b, false), b, vpPrecLimit(context.runtime));
return addInternal(context, getVpValueWithPrec(context, b, false), b, vpPrecLimit(context.runtime));
}

@Deprecated
Expand All @@ -1046,7 +1047,7 @@ public IRubyObject op_plus19(ThreadContext context, IRubyObject b) {

@JRubyMethod(name = "add")
public IRubyObject add2(ThreadContext context, IRubyObject b, IRubyObject digits) {
return addInternal(context, getVpValue19(context, b, false), b, digits);
return addInternal(context, getVpValueWithPrec(context, b, false), b, digits);
}

@Deprecated
Expand Down Expand Up @@ -1131,7 +1132,7 @@ public IRubyObject op_uminus(ThreadContext context) {

@JRubyMethod(name = "-", required = 1)
public IRubyObject op_minus(ThreadContext context, IRubyObject b) {
return subInternal(context, getVpValue19(context, b, true), b, 0);
return subInternal(context, getVpValueWithPrec(context, b, true), b, 0);
}

@Deprecated
Expand All @@ -1141,7 +1142,7 @@ public IRubyObject op_minus19(ThreadContext context, IRubyObject b) {

@JRubyMethod(name = "sub", required = 2)
public IRubyObject sub2(ThreadContext context, IRubyObject b, IRubyObject n) {
return subInternal(context, getVpValue19(context, b, false), b, getPositiveInt(context, n));
return subInternal(context, getVpValueWithPrec(context, b, false), b, getPositiveInt(context, n));
}

@Deprecated
Expand Down Expand Up @@ -1186,7 +1187,7 @@ private RubyBigDecimal subSpecialCases(ThreadContext context, RubyBigDecimal val

@JRubyMethod(name = {"/", "quo"})
public IRubyObject op_quo(ThreadContext context, IRubyObject other) {
RubyBigDecimal val = getVpValue19(context, other, false);
RubyBigDecimal val = getVpValueWithPrec(context, other, false);
if (val == null) return callCoerced(context, sites(context).op_quo, other, true);

if (isNaN() || val.isNaN()) return newNaN(context.runtime);
Expand All @@ -1198,17 +1199,31 @@ public IRubyObject op_quo(ThreadContext context, IRubyObject other) {
}

private RubyBigDecimal quoImpl(ThreadContext context, RubyBigDecimal that) {
// regular division with some default precision
// proper algorithm to set the precision
// the precision is multiple of 4
// and the precision is larger than len * 2
int len = this.value.precision() + that.value.precision();
int scale = (len / 4 + 1) * 4 * 2;
int mx = this.value.precision();
int mxb = that.value.precision();

MathContext mathContext = new MathContext(scale, getRoundingMode(context.runtime));
if (mx < mxb) mx = mxb;
mx = (mx + 1) * BASE_FIG;

MathContext mathContext = new MathContext(mx, getRoundingMode(context.runtime));
return new RubyBigDecimal(context.runtime, this.value.divide(that.value, mathContext)).setResult();
}

private static RubyBigDecimal div2Impl(ThreadContext context, RubyNumeric a, RubyNumeric b, final int ix) {
RubyBigDecimal thiz = getVpValue(context, a, true);
RubyBigDecimal that = getVpValue(context, b, true);

if (thiz.isNaN() || that.isNaN()) return newNaN(context.runtime);

RubyBigDecimal div = thiz.divSpecialCases(context, that);
if (div != null) return div;

int mx = thiz.value.precision() + that.value.precision() + 2;

MathContext mathContext = new MathContext((mx * 2 + 2) * BASE_FIG, getRoundingMode(context.runtime));
return new RubyBigDecimal(context.runtime, thiz.value.divide(that.value, mathContext)).setResult(ix);
}

@Deprecated
public IRubyObject op_quo19(ThreadContext context, IRubyObject other) {
return op_quo(context, other);
Expand Down Expand Up @@ -1450,7 +1465,7 @@ public RubyNumeric multiplyWith(ThreadContext context, RubyBignum value) {
public IRubyObject divmod(ThreadContext context, IRubyObject other) {
final Ruby runtime = context.runtime;

RubyBigDecimal val = getVpValue19(context, other, false);
RubyBigDecimal val = getVpValueWithPrec(context, other, false);
if (val == null) return callCoerced(context, sites(context).divmod, other, true);

if (isNaN() || val.isNaN() || isInfinity() && val.isInfinity()) return RubyArray.newArray(runtime, newNaN(runtime), newNaN(runtime));
Expand Down
98 changes: 96 additions & 2 deletions test/jruby/test_big_decimal.rb
View file
Expand Up @@ -234,8 +234,8 @@ def teardown
def test_big_decimal_mode
# Accept valid arguments to #mode
assert BigDecimal.mode(BigDecimal::EXCEPTION_OVERFLOW)
assert BigDecimal.mode(BigDecimal::EXCEPTION_OVERFLOW,true)
assert BigDecimal.mode(BigDecimal::EXCEPTION_OVERFLOW,false)
assert BigDecimal.mode(BigDecimal::EXCEPTION_OVERFLOW, true)
assert BigDecimal.mode(BigDecimal::EXCEPTION_OVERFLOW, false)

# Reject invalid arguments to #mode
assert_raises(TypeError) { BigDecimal.mode(true) } # first argument must be a Fixnum
Expand Down Expand Up @@ -388,6 +388,35 @@ def test_div_returns_integer_by_default
assert_equal 999900009999000099990, res
end

def test_div_mult_precision
small = BigDecimal("1E-99"); denom = BigDecimal('0.000000123')
res = small / denom
# MRI (2.5):
# 0.8130081300813008130081300813008130081300813008130081300813008130081300813008130081300813008130081e-92
puts "\n#{__method__} #{small} / #{denom} = #{res} (#{res.precs})" if $VERBOSE
assert res.to_s.length > 40, "not enough precision: #{res}" # in JRuby 9.1 only 20
assert_equal BigDecimal('0.81300813e-92'), res.round(100)

val1 = BigDecimal('1'); val2 = BigDecimal('3')
res = val1 / val2
puts "\n#{__method__} #{val1} / #{val2} = #{res} (#{res.precs})" if $VERBOSE
assert 18 <= res.precs.first, "unexpected precision: #{res.precs.first}"
assert res.precs.first <= 20, "unexpected precision: #{res.precs.first}"
assert_equal '0.333333333333333333e0', res.to_s

val1 = BigDecimal("1.00000000000000000001"); val2 = Rational(1, 3)
res = val1 * val2
puts "\n#{__method__} #{val1} * #{val2} = #{res} (#{res.precs})" if $VERBOSE
assert res.to_s.start_with?('0.333333333333333333336666666666666666'), "invalid result: #{res.to_s}"
# MRI (2.5):
# 0.33333333333333333333666666666666666633333333333333333333e0

# test_div from MRI's suite :
assert_equal(BigDecimal('1486.868686869'), (BigDecimal('1472.0') / BigDecimal('0.99')).round(9))

assert_equal(4.124045235, (BigDecimal('0.9932') / (700 * BigDecimal('0.344045') / BigDecimal('1000.0'))).round(9))
end

def test_zero_p # from MRI test_bigdecimal.rb
# BigDecimal.mode(BigDecimal::EXCEPTION_INFINITY, false)
# BigDecimal.mode(BigDecimal::EXCEPTION_NaN, false)
Expand All @@ -408,4 +437,69 @@ def test_power_of_three # from MRI test_bigdecimal.rb
assert_in_delta(1.0/81, x ** -4)
end

def test_div # from MRI test_bigdecimal.rb
x = BigDecimal((2**100).to_s)
assert_equal(BigDecimal((2**100 / 3).to_s), (x / 3).to_i)
assert_equal(BigDecimal::SIGN_POSITIVE_ZERO, (BigDecimal("0") / 1).sign)
assert_equal(BigDecimal::SIGN_NEGATIVE_ZERO, (BigDecimal("-0") / 1).sign)
assert_equal(2, BigDecimal("2") / 1)
assert_equal(-2, BigDecimal("2") / -1)

#assert_equal(BigDecimal('1486.868686869'), BigDecimal('1472.0') / BigDecimal('0.99'), '[ruby-core:59365] [#9316]')

#assert_equal(4.124045235, BigDecimal('0.9932') / (700 * BigDecimal('0.344045') / BigDecimal('1000.0')), '[#9305]')

BigDecimal.mode(BigDecimal::EXCEPTION_INFINITY, false)
assert_positive_zero(BigDecimal("1.0") / BigDecimal("Infinity"))
assert_negative_zero(BigDecimal("-1.0") / BigDecimal("Infinity"))
assert_negative_zero(BigDecimal("1.0") / BigDecimal("-Infinity"))
assert_positive_zero(BigDecimal("-1.0") / BigDecimal("-Infinity"))

# BigDecimal.mode(BigDecimal::EXCEPTION_INFINITY, true)
# BigDecimal.mode(BigDecimal::EXCEPTION_ZERODIVIDE, false)
# assert_raise_with_message(FloatDomainError, "Computation results to 'Infinity'") { BigDecimal("1") / 0 }
# assert_raise_with_message(FloatDomainError, "Computation results to '-Infinity'") { BigDecimal("-1") / 0 }
end

def test_new # from MRI test_bigdecimal.rb
assert_equal(1, BigDecimal("1"))
assert_equal(1, BigDecimal("1", 1))
assert_equal(1, BigDecimal(" 1 "))
assert_equal(111, BigDecimal("1_1_1_"))
assert_equal(10**(-1), BigDecimal("1E-1"), '#4825')

#assert_raise(ArgumentError, /"_1_1_1"/) { BigDecimal("_1_1_1") }

BigDecimal.mode(BigDecimal::EXCEPTION_OVERFLOW, false)
BigDecimal.mode(BigDecimal::EXCEPTION_NaN, false)
assert_positive_infinite(BigDecimal("Infinity"))
assert_negative_infinite(BigDecimal("-Infinity"))
assert_nan(BigDecimal("NaN"))
assert_positive_infinite(BigDecimal("1E1111111111111111111"))
end

private

def assert_nan(x)
assert(x.nan?, "Expected #{x.inspect} to be NaN")
end

def assert_positive_infinite(x)
assert(x.infinite?, "Expected #{x.inspect} to be positive infinite")
assert_operator(x, :>, 0)
end

def assert_negative_infinite(x)
assert(x.infinite?, "Expected #{x.inspect} to be negative infinite")
assert_operator(x, :<, 0)
end

def assert_positive_zero(x)
assert_equal(BigDecimal::SIGN_POSITIVE_ZERO, x.sign, "Expected #{x.inspect} to be positive zero")
end

def assert_negative_zero(x)
assert_equal(BigDecimal::SIGN_NEGATIVE_ZERO, x.sign, "Expected #{x.inspect} to be negative zero")
end

end
7 changes: 3 additions & 4 deletions test/mri/excludes/TestBigDecimal.rb
View file
Expand Up @@ -2,13 +2,12 @@

exclude :test_BigMath_exp_under_gc_stress, "needs investigation"
exclude :test_BigMath_log_under_gc_stress, "needs investigation"
exclude :test_div, "needs investigation"
exclude :test_exception_underflow, "needs investigation"
exclude :test_div, "does not pass due precision differences (ported to test/jruby/test_big_decimal.rb)"

exclude :test_global_new_with_invalid_string, "error change in 2.4?"
exclude :test_limit, "needs investigation"
exclude :test_marshal, "needs investigation"
exclude :test_new, "stricter parsing in 2.4"
exclude :test_new, "BigDecimal('_1_1_1') parses fine while in MRI raises"

exclude :test_power_of_negative_infinity, "needs investigation"
exclude :test_power_of_positive_infinity, "needs investigation"
Expand All @@ -22,6 +21,6 @@
exclude :test_sqrt_bigdecimal, "needs investigation"
exclude :test_thread_local_mode, "needs investigation"
exclude :test_to_f, "needs investigation"
exclude :test_to_special_string, "needs investigation"
exclude :test_to_special_string, "these are really not that relevant - a bit weird behavior"
exclude :test_zero_p, "200000000000000 is too big for Java int exponent"
exclude :test_ctov, "needs investigation"

0 comments on commit 2b06a12

Please sign in to comment.