From 139d4a87b464071cf9d97df3d2b9cd877ee56887 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 10 Mar 2023 12:26:51 +0530 Subject: [PATCH] fix: corrected banker's rounding closes https://github.com/frappe/frappe/issues/19570 --- cypress/integration/rounding.js | 62 +++++++++++++++- .../system_settings/system_settings.json | 2 +- .../public/js/frappe/utils/number_format.js | 19 +++++ frappe/tests/test_utils.py | 71 +++++++++++++++++++ frappe/utils/data.py | 22 +++++- 5 files changed, 172 insertions(+), 4 deletions(-) diff --git a/cypress/integration/rounding.js b/cypress/integration/rounding.js index daad2def8f..1a9cfa685b 100644 --- a/cypress/integration/rounding.js +++ b/cypress/integration/rounding.js @@ -4,7 +4,7 @@ context("Rounding behaviour", () => { cy.visit("/app/"); }); - it("Rounds floats accurately", () => { + it("Commercial Rounding", () => { cy.window() .its("flt") .then((flt) => { @@ -41,4 +41,64 @@ context("Rounding behaviour", () => { expect(flt(-0.15, 1, null, rounding_method)).eq(-0.2); }); }); + + it("Banker's Rounding", () => { + cy.window() + .its("flt") + .then((flt) => { + let rounding_method = "Banker's Rounding"; + + expect(flt("0.5", 0, null, rounding_method)).eq(0); + expect(flt("0.3", null, rounding_method)).eq(0.3); + + expect(flt("1.5", 0, null, rounding_method)).eq(2); + + // positive rounding to integers + expect(flt(0.4, 0, null, rounding_method)).eq(0); + expect(flt(0.5, 0, null, rounding_method)).eq(0); + expect(flt(1.455, 0, null, rounding_method)).eq(1); + expect(flt(1.5, 0, null, rounding_method)).eq(2); + + // negative rounding to integers + expect(flt(-0.5, 0, null, rounding_method)).eq(0); + expect(flt(-1.5, 0, null, rounding_method)).eq(-2); + + // negative precision i.e. round to nearest 10th + expect(flt(123, -1, null, rounding_method)).eq(120); + expect(flt(125, -1, null, rounding_method)).eq(120); + expect(flt(134.45, -1, null, rounding_method)).eq(130); + expect(flt(135, -1, null, rounding_method)).eq(140); + + // positive multiple digit rounding + expect(flt(1.25, 1, null, rounding_method)).eq(1.2); + expect(flt(0.15, 1, null, rounding_method)).eq(0.2); + expect(flt(2.675, 2, null, rounding_method)).eq(2.68); + expect(flt(-2.675, 2, null, rounding_method)).eq(-2.68); + + // negative multiple digit rounding + expect(flt(-1.25, 1, null, rounding_method)).eq(-1.2); + expect(flt(-0.15, 1, null, rounding_method)).eq(-0.2); + + // Nearest number and not even (the default behaviour) + expect(flt(0.5, 0, null, rounding_method)).eq(0); + expect(flt(1.5, 0, null, rounding_method)).eq(2); + expect(flt(2.5, 0, null, rounding_method)).eq(2); + expect(flt(3.5, 0, null, rounding_method)).eq(4); + + expect(flt(0.05, 1, null, rounding_method)).eq(0.0); + expect(flt(1.15, 1, null, rounding_method)).eq(1.2); + expect(flt(2.25, 1, null, rounding_method)).eq(2.2); + expect(flt(3.35, 1, null, rounding_method)).eq(3.4); + + expect(flt(-0.5, 0, null, rounding_method)).eq(0); + expect(flt(-1.5, 0, null, rounding_method)).eq(-2); + expect(flt(-2.5, 0, null, rounding_method)).eq(-2); + expect(flt(-3.5, 0, null, rounding_method)).eq(-4); + + expect(flt(-0.05, 1, null, rounding_method)).eq(0.0); + expect(flt(-1.15, 1, null, rounding_method)).eq(-1.2); + expect(flt(-2.25, 1, null, rounding_method)).eq(-2.2); + expect(flt(-3.35, 1, null, rounding_method)).eq(-3.4); + }); + }); }); diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 642e454717..8ebcb493de 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -527,7 +527,7 @@ "fieldname": "rounding_method", "fieldtype": "Select", "label": "Rounding Method", - "options": "Banker's Rounding (legacy)\nCommercial Rounding" + "options": "Banker's Rounding (legacy)\nBanker's Rounding\nCommercial Rounding" } ], "icon": "fa fa-cog", diff --git a/frappe/public/js/frappe/utils/number_format.js b/frappe/public/js/frappe/utils/number_format.js index 2c520805f2..179c186908 100644 --- a/frappe/public/js/frappe/utils/number_format.js +++ b/frappe/public/js/frappe/utils/number_format.js @@ -188,6 +188,25 @@ function _round(num, precision, rounding_method) { var r = !precision && f == 0.5 ? (i % 2 == 0 ? i : i + 1) : Math.round(n); r = d ? r / m : r; return is_negative ? -r : r; + } else if (rounding_method == "Banker's Rounding") { + precision = cint(precision); + + let multiplier = Math.pow(10, precision); + num = Math.abs(num) * multiplier; + + let floor_num = Math.floor(num); + let decimal_part = num - floor_num; + + // For explanation of this method read python flt implementation notes. + let epsilon = 2.0 ** (Math.log2(Math.abs(num)) - 52.0); + + if (Math.abs(decimal_part - 0.5) < epsilon) { + num = floor_num % 2 == 0 ? floor_num : floor_num + 1; + } else { + num = Math.round(num); + } + num = num / multiplier; + return is_negative ? -num : num; } else if (rounding_method == "Commercial Rounding") { if (num == 0) return 0.0; diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 52e91b9500..b897c96133 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -1073,9 +1073,80 @@ class TestRounding(FrappeTestCase): self.assertEqual(flt(-1.25, 1, rounding_method=rounding_method), -1.3) self.assertEqual(flt(-0.15, 1, rounding_method=rounding_method), -0.2) + # Nearest number and not even (the default behaviour) + self.assertEqual(flt(0.5, 0, rounding_method=rounding_method), 1) + self.assertEqual(flt(1.5, 0, rounding_method=rounding_method), 2) + self.assertEqual(flt(2.5, 0, rounding_method=rounding_method), 3) + self.assertEqual(flt(3.5, 0, rounding_method=rounding_method), 4) + + self.assertEqual(flt(0.05, 1, rounding_method=rounding_method), 0.1) + self.assertEqual(flt(1.15, 1, rounding_method=rounding_method), 1.2) + self.assertEqual(flt(2.25, 1, rounding_method=rounding_method), 2.3) + self.assertEqual(flt(3.35, 1, rounding_method=rounding_method), 3.4) + @change_settings("System Settings", {"rounding_method": "Commercial Rounding"}) @given(st.decimals(min_value=-1e8, max_value=1e8), st.integers(min_value=-2, max_value=4)) def test_normal_rounding_property(self, number, precision): with localcontext() as ctx: ctx.rounding = ROUND_HALF_UP self.assertEqual(Decimal(str(flt(float(number), precision))), round(number, precision)) + + def test_bankers_rounding(self): + rounding_method = "Banker's Rounding" + + self.assertEqual(flt("0.5", 0, rounding_method=rounding_method), 0) + self.assertEqual(flt("0.3", rounding_method=rounding_method), 0.3) + + self.assertEqual(flt("1.5", 0, rounding_method=rounding_method), 2) + + # positive rounding to integers + self.assertEqual(flt(0.4, 0, rounding_method=rounding_method), 0) + self.assertEqual(flt(0.5, 0, rounding_method=rounding_method), 0) + self.assertEqual(flt(1.455, 0, rounding_method=rounding_method), 1) + self.assertEqual(flt(1.5, 0, rounding_method=rounding_method), 2) + + # negative rounding to integers + self.assertEqual(flt(-0.5, 0, rounding_method=rounding_method), 0) + self.assertEqual(flt(-1.5, 0, rounding_method=rounding_method), -2) + + # negative precision i.e. round to nearest 10th + self.assertEqual(flt(123, -1, rounding_method=rounding_method), 120) + self.assertEqual(flt(125, -1, rounding_method=rounding_method), 120) + self.assertEqual(flt(134.45, -1, rounding_method=rounding_method), 130) + self.assertEqual(flt(135, -1, rounding_method=rounding_method), 140) + + # positive multiple digit rounding + self.assertEqual(flt(1.25, 1, rounding_method=rounding_method), 1.2) + self.assertEqual(flt(0.15, 1, rounding_method=rounding_method), 0.2) + self.assertEqual(flt(2.675, 2, rounding_method=rounding_method), 2.68) + self.assertEqual(flt(-2.675, 2, rounding_method=rounding_method), -2.68) + + # negative multiple digit rounding + self.assertEqual(flt(-1.25, 1, rounding_method=rounding_method), -1.2) + self.assertEqual(flt(-0.15, 1, rounding_method=rounding_method), -0.2) + + # Nearest number and not even (the default behaviour) + self.assertEqual(flt(0.5, 0, rounding_method=rounding_method), 0) + self.assertEqual(flt(1.5, 0, rounding_method=rounding_method), 2) + self.assertEqual(flt(2.5, 0, rounding_method=rounding_method), 2) + self.assertEqual(flt(3.5, 0, rounding_method=rounding_method), 4) + + self.assertEqual(flt(0.05, 1, rounding_method=rounding_method), 0.0) + self.assertEqual(flt(1.15, 1, rounding_method=rounding_method), 1.2) + self.assertEqual(flt(2.25, 1, rounding_method=rounding_method), 2.2) + self.assertEqual(flt(3.35, 1, rounding_method=rounding_method), 3.4) + + self.assertEqual(flt(-0.5, 0, rounding_method=rounding_method), 0) + self.assertEqual(flt(-1.5, 0, rounding_method=rounding_method), -2) + self.assertEqual(flt(-2.5, 0, rounding_method=rounding_method), -2) + self.assertEqual(flt(-3.5, 0, rounding_method=rounding_method), -4) + + self.assertEqual(flt(-0.05, 1, rounding_method=rounding_method), 0.0) + self.assertEqual(flt(-1.15, 1, rounding_method=rounding_method), -1.2) + self.assertEqual(flt(-2.25, 1, rounding_method=rounding_method), -2.2) + self.assertEqual(flt(-3.35, 1, rounding_method=rounding_method), -3.4) + + @change_settings("System Settings", {"rounding_method": "Banker's Rounding"}) + @given(st.decimals(min_value=-1e8, max_value=1e8), st.integers(min_value=-2, max_value=4)) + def test_bankers_rounding_property(self, number, precision): + self.assertEqual(Decimal(str(flt(float(number), precision))), round(number, precision)) diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 8a6037d718..26fb50c31b 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -1059,7 +1059,9 @@ def rounded(num, precision=0, rounding_method=None): ) if rounding_method == "Banker's Rounding (legacy)": - return _round_half_even(num, precision) + return _bankers_rounding_legacy(num, precision) + elif rounding_method == "Banker's Rounding": + return _bankers_rounding(num, precision) elif rounding_method == "Commercial Rounding": return _round_away_from_zero(num, precision) else: @@ -1069,7 +1071,7 @@ def rounded(num, precision=0, rounding_method=None): ) -def _round_half_even(num, precision): +def _bankers_rounding_legacy(num, precision): # avoid rounding errors multiplier = 10**precision num = round(num * multiplier if precision else num, 8) @@ -1114,6 +1116,22 @@ def _round_away_from_zero(num, precision): return round(num + math.copysign(epsilon, num), precision) +def _bankers_rounding(num, precision): + multiplier = 10**precision + num = round(num * multiplier, 12) + + floor_num = math.floor(num) + decimal_part = num - floor_num + + epsilon = 2.0 ** (math.log(abs(num), 2) - 52.0) + if abs(decimal_part - 0.5) < epsilon: + num = floor_num if (floor_num % 2 == 0) else floor_num + 1 + else: + num = round(num) + + return num / multiplier + + def remainder(numerator: NumericType, denominator: NumericType, precision: int = 2) -> NumericType: precision = cint(precision) multiplier = 10**precision