Skip to content

fix(round): round negative Fractions half away from zero - #3702

Open
yu2971512385-ui wants to merge 1 commit into
josdejong:developfrom
yu2971512385-ui:fix/round-fraction-half-away-from-zero
Open

yu2971512385-ui wants to merge 1 commit into
josdejong:developfrom
yu2971512385-ui:fix/round-fraction-half-away-from-zero

Conversation

@yu2971512385-ui

Copy link
Copy Markdown

round resolves a tie differently depending on the type of the value:

math.round(-2.5)                     // -3
math.round(math.bignumber(-2.5))     // -3
math.round(math.fraction(-2.5))      // -2   <-

math.round(-0.25, 1)                 // -0.3
math.round(math.bignumber(-0.25), 1) // -0.3
math.round(math.fraction(-0.25), 1)  // -0.2 <-

number and BigNumber round a half away from zero (roundNumber, and decimal.js's default ROUND_HALF_UP which rounds away from zero). Fraction.round follows Math.round instead, which rounds a half towards positive infinity, so every exact half with a negative fraction lands on the other side. Positive values agree across all three types, which is why this is easy to miss.

floor, ceil and fix are already consistent across the three types; round is the one that is not.

Change

roundFraction rounds the magnitude and re-applies the sign, and the three Fraction signatures use it. Positive fractions are untouched, and the existing behaviour for number, bigint, BigNumber, Unit and matrices is unchanged.

Tests

A new case in test/unit-tests/function/arithmetic/round.test.js covers -1/2, -3/2, -5/2, -2/3, -1/3, and -1/4 with 1 decimal (as number and as BigNumber), and asserts the same expectations against the number and BigNumber implementations so the three stay pinned together. The new test fails before the change; npx mocha test/unit-tests --recursive passes after it (6653 passing), and eslint is clean.

Note on Complex

Complex.round has the same tie behaviour (math.round(math.complex(-2.5, -2.5)) → -2 - 2i), since complex.js also uses Math.round. Fixing that means rounding the parts in mathjs rather than delegating, which changes more than this fix — happy to follow up with a separate PR if you would like that made consistent too.

round() resolved a tie differently depending on the value's type:
number and BigNumber round a half away from zero, while Fraction (and
so also 'round(fraction(x), n)') rounds it towards positive infinity,
because Fraction.round follows Math.round:

    math.round(-2.5)                  // -3
    math.round(math.bignumber(-2.5))  // -3
    math.round(math.fraction(-2.5))   // -2  <-
    math.round(-0.25, 1)              // -0.3
    math.round(math.fraction(-0.25), 1) // -0.2  <-

Round the magnitude and re-apply the sign so all three types agree.
Positive fractions are unaffected.

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