Skip to content

Round away from zero in ROUND_UP for values far below the rounding position - #560

Open
Shaisolaris wants to merge 1 commit into
ruby:masterfrom
Shaisolaris:round-up-small-values
Open

Shaisolaris wants to merge 1 commit into
ruby:masterfrom
Shaisolaris:round-up-small-values

Conversation

@Shaisolaris

Copy link
Copy Markdown

round(n, :up) returns zero for a nonzero value that lies more than one internal digit word (9 decimal digits) below the rounding position:

BigDecimal("1e-7").round(0, :up)    # => 1
BigDecimal("1e-10").round(0, :up)   # => 0, expected 1
BigDecimal("1e-10").round(6, :up)   # => 0.0, expected 0.000001
BigDecimal("1e-10").ceil(6)         # => 0.000001

VpMidRound truncates everything when the rounding position is left of the first word, but keeps going for ROUND_CEILING and ROUND_FLOOR so they can round to the next unit. ROUND_UP needs the same treatment, since any nonzero remainder rounds away from zero. The change adds it to that check; the existing path (exptoadd) then produces the right result.

Testing:

  • New test_round_up_value_far_below_rounding_position fails before the change and passes after.
  • All files under test/bigdecimal pass with the change (270 tests, 0 failures, 13 omissions) on Ruby 3.3.8, Linux x86_64. test_sig and JRuby were not run.
  • A randomized comparison of round(n, mode) against exact Rational rounding for all seven modes (60,000 cases, exponents -60..40, n -45..45) found 4,800 :up mismatches before and none after.

…sition

VpMidRound zeroed the value whenever the rounding position was left of
the first digit word, except for ROUND_CEILING and ROUND_FLOOR. ROUND_UP
also rounds any nonzero remainder away from zero, so it gave 0 instead of
the next unit, e.g. BigDecimal("1e-10").round(0, :up) returned 0 while
BigDecimal("1e-7").round(0, :up) returned 1.

This branch has not been deployed

No deployments
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 this pull request may close these issues.

1 participant