Add a precision assertion to BigMath test #316
Conversation
0b138e4 to
c1dfce8
Compare
c1dfce8 to
395fb3b
Compare
test/bigdecimal/test_bigmath.rb
Outdated
| precision = -(value / expected - 1).exponent | ||
| assert(value != expected, "Unable to estimate precision for exact value") | ||
| assert(precision >= n, "Precision is not enough: #{precision} < #{n}") | ||
| end |
There was a problem hiding this comment.
These two assertions seem useful in test_bigdecimal.rb too. Why not relocate them to helper.rb?
test/bigdecimal/test_bigmath.rb
Outdated
| @@ -6,10 +6,30 @@ class TestBigMath < Test::Unit::TestCase | |||
| include TestBigDecimalBase | |||
| include BigMath | |||
| N = 20 | |||
| N_LARGE = 100 | |||
There was a problem hiding this comment.
This is just a comment to share my concern regarding the use of BigDecimal on 64-bit architectures. Each BigDecimal digit element can hold 19 decimal digits, meaning that 100 would require 6 BigDecimal digits. We should consider if 6 BigDecimal digits is truly a large number.
There was a problem hiding this comment.
Update the precision assertion to use [50, 100, 150]
Trying multiple precisions will increase the probability of detecting mis-calculation.
test/bigdecimal/test_bigmath.rb
Outdated
| def assert_relative_precision(n = N_LARGE) | ||
| value = yield(n) | ||
| expected = yield(2 * n) | ||
| precision = -(value / expected - 1).exponent |
There was a problem hiding this comment.
Don’t you have to specify the quotient precision explicitly in the division?
There was a problem hiding this comment.
Changed to value.div(expected, 2 * n)
Precision assertion in BigMath test is missing.
BigMath uses
BigDecimal.double_fig(16 in my env), so the calculated precision is normally >=16.We need to test with precision several times larger than 16 to ensure that the result is really precise enough.
This assertion will be helpful when adding new methods to BigMath (example:
BigMath.tan)Precision check
Assume
BigMath.func(v, 2 * prec)to be the expected result which is precise enough and calculates the difference.assert_fixed_point_precisionis for sin and cos.assert_relative_precisionis for other methods.sin and cos
I think
BigMath.sin(x),BigMath.cos(x)with largexis not precise enough. This test case is not added in this pull request.