diff --git a/cypress/integration/rounding.js b/cypress/integration/rounding.js new file mode 100644 index 0000000000..647b32a6a5 --- /dev/null +++ b/cypress/integration/rounding.js @@ -0,0 +1,44 @@ +context("Rounding behaviour", () => { + before(() => { + cy.login(); + cy.visit("/app/"); + }); + + it("Rounds floats accurately", () => { + cy.window() + .its("flt") + .then((flt) => { + let rounding_method = "Rounding Half Away From Zero"; + + expect(flt("0.5", 0, null, rounding_method)).eq(1); + expect(flt("0.3", null, 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(1); + 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(-1); + 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(130); + 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.3); + expect(flt(0.15, 1, null, rounding_method)).eq(0.2); + 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.3); + expect(flt(-0.15, 1, null, rounding_method)).eq(-0.2); + }); + }); +}); diff --git a/frappe/core/doctype/system_settings/system_settings.js b/frappe/core/doctype/system_settings/system_settings.js index 1de4dd82e7..1d5ba7ddb0 100644 --- a/frappe/core/doctype/system_settings/system_settings.js +++ b/frappe/core/doctype/system_settings/system_settings.js @@ -39,4 +39,21 @@ frappe.ui.form.on("System Settings", { first_day_of_the_week(frm) { frm.re_setup_moment = true; }, + + rounding_method: function (frm) { + if (frm.doc.rounding_method == frappe.boot.sysdefaults.rounding_method) return; + let msg = __( + "Changing rounding method on site with data can result in unexpected behaviour." + ); + msg += "
"; + msg += __("Do you still want to proceed?"); + + frappe.confirm( + msg, + () => {}, + () => { + frm.set_value("rounding_method", frappe.boot.sysdefaults.rounding_method); + } + ); + }, }); diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index ddafd0e9fd..2c9e92d943 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -17,10 +17,11 @@ "date_format", "time_format", "number_format", + "first_day_of_the_week", "column_break_7", "float_precision", "currency_precision", - "first_day_of_the_week", + "rounding_method", "sec_backup_limit", "backup_limit", "encrypt_backup", @@ -520,12 +521,19 @@ "fieldname": "login_with_email_link_expiry", "fieldtype": "Int", "label": "Login with email link expiry (in minutes)" + }, + { + "default": "Round Half Even", + "fieldname": "rounding_method", + "fieldtype": "Select", + "label": "Rounding Method", + "options": "Round Half Even\nRounding Half Away From Zero" } ], "icon": "fa fa-cog", "issingle": 1, "links": [], - "modified": "2022-12-20 21:45:37.651668", + "modified": "2023-03-06 11:31:19.144956", "modified_by": "Administrator", "module": "Core", "name": "System Settings", @@ -544,4 +552,4 @@ "sort_order": "ASC", "states": [], "track_changes": 1 -} \ No newline at end of file +} diff --git a/frappe/exceptions.py b/frappe/exceptions.py index 20e858c543..26c323352d 100644 --- a/frappe/exceptions.py +++ b/frappe/exceptions.py @@ -273,6 +273,10 @@ class ExecutableNotFound(FileNotFoundError): pass +class InvalidRoundingMethod(FileNotFoundError): + pass + + class InvalidRemoteException(Exception): pass diff --git a/frappe/public/js/frappe/utils/number_format.js b/frappe/public/js/frappe/utils/number_format.js index 63b0cbe451..2c457e75fe 100644 --- a/frappe/public/js/frappe/utils/number_format.js +++ b/frappe/public/js/frappe/utils/number_format.js @@ -5,7 +5,7 @@ import "./datatype"; if (!window.frappe) window.frappe = {}; -function flt(v, decimals, number_format) { +function flt(v, decimals, number_format, rounding_method) { if (v == null || v == "") return 0; if (!(typeof v === "number" || String(parseFloat(v)) == v)) { @@ -30,7 +30,7 @@ function flt(v, decimals, number_format) { } v = parseFloat(v); - if (decimals != null) return _round(v, decimals); + if (decimals != null) return _round(v, decimals, rounding_method); return v; } @@ -173,16 +173,40 @@ function get_number_format_info(format) { return info; } -function _round(num, precision) { - var is_negative = num < 0 ? true : false; - var d = cint(precision); - var m = Math.pow(10, d); - var n = +(d ? Math.abs(num) * m : Math.abs(num)).toFixed(8); // Avoid rounding errors - var i = Math.floor(n), - f = n - i; - 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; +function _round(num, precision, rounding_method) { + rounding_method = + rounding_method || frappe.boot.sysdefaults.rounding_method || "Round Half Even"; + + let is_negative = num < 0 ? true : false; + + if (rounding_method == "Round Half Even") { + var d = cint(precision); + var m = Math.pow(10, d); + var n = +(d ? Math.abs(num) * m : Math.abs(num)).toFixed(8); // Avoid rounding errors + var i = Math.floor(n), + f = n - i; + 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 == "Rounding Half Away From Zero") { + if (num == 0) return 0.0; + + let digits = cint(precision); + let multiplier = Math.pow(10, digits); + + num = num * multiplier; + + // For explanation of this method read python flt implementation notes. + let epsilon = 2.0 ** (Math.log2(Math.abs(num)) - 52.0); + if (is_negative) { + epsilon = -1 * epsilon; + } + + num = Math.round(num + epsilon); + return num / multiplier; + } else { + throw new Error(`Unknown rounding method ${rounding_method}`); + } } function roundNumber(num, precision) { diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index d2d5cdafd7..ce13ee65cd 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -6,25 +6,28 @@ import json import os import sys from datetime import date, datetime, time, timedelta -from decimal import Decimal +from decimal import ROUND_HALF_UP, Decimal, localcontext from enum import Enum from io import StringIO from mimetypes import guess_type from unittest.mock import patch import pytz +from hypothesis import given +from hypothesis import strategies as st from PIL import Image import frappe from frappe.installer import parse_app_name from frappe.model.document import Document -from frappe.tests.utils import FrappeTestCase +from frappe.tests.utils import FrappeTestCase, change_settings from frappe.utils import ( ceil, dict_to_str, evaluate_filters, execute_in_shell, floor, + flt, format_timedelta, get_bench_path, get_file_timestamp, @@ -1001,3 +1004,78 @@ class TestTBSanitization(FrappeTestCase): self.assertIn("********", traceback) self.assertIn("password =", traceback) self.assertIn("safe_value", traceback) + + +class TestRounding(FrappeTestCase): + @change_settings("System Settings", {"rounding_method": "Rounding Half Away From Zero"}) + def test_normal_rounding(self): + self.assertEqual(flt("what"), 0) + + self.assertEqual(flt("0.5", 0), 1) + self.assertEqual(flt("0.3"), 0.3) + + self.assertEqual(flt("1.5", 0), 2) + + # positive rounding to integers + self.assertEqual(flt(0.4, 0), 0) + self.assertEqual(flt(0.5, 0), 1) + self.assertEqual(flt(1.455, 0), 1) + self.assertEqual(flt(1.5, 0), 2) + + # negative rounding to integers + self.assertEqual(flt(-0.5, 0), -1) + self.assertEqual(flt(-1.5, 0), -2) + + # negative precision i.e. round to nearest 10th + self.assertEqual(flt(123, -1), 120) + self.assertEqual(flt(125, -1), 130) + self.assertEqual(flt(134.45, -1), 130) + self.assertEqual(flt(135, -1), 140) + + # positive multiple digit rounding + self.assertEqual(flt(1.25, 1), 1.3) + self.assertEqual(flt(0.15, 1), 0.2) + + # negative multiple digit rounding + self.assertEqual(flt(-1.25, 1), -1.3) + self.assertEqual(flt(-0.15, 1), -0.2) + + def test_normal_rounding_as_argument(self): + rounding_method = "Rounding Half Away From Zero" + + self.assertEqual(flt("0.5", 0, rounding_method=rounding_method), 1) + 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), 1) + 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), -1) + 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), 130) + 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.3) + self.assertEqual(flt(0.15, 1, rounding_method=rounding_method), 0.2) + 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.3) + self.assertEqual(flt(-0.15, 1, rounding_method=rounding_method), -0.2) + + @change_settings("System Settings", {"rounding_method": "Rounding Half Away From Zero"}) + @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)) diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 92467b036b..eeae737144 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -920,7 +920,9 @@ def flt(s: NumericType | str, precision: int | None = None) -> float: ... -def flt(s: NumericType | str, precision: int | None = None) -> float: +def flt( + s: NumericType | str, precision: int | None = None, rounding_method: str | None = None +) -> float: """Convert to float (ignoring commas in string) :param s: Number in string or other numeric format. @@ -946,8 +948,10 @@ def flt(s: NumericType | str, precision: int | None = None) -> float: try: num = float(s) if precision is not None: - num = rounded(num, precision) - except Exception: + num = rounded(num, precision, rounding_method) + except Exception as e: + if isinstance(e, frappe.InvalidRoundingMethod): + raise num = 0.0 return num @@ -1046,12 +1050,28 @@ def sbool(x: str) -> bool | Any: return x -def rounded(num, precision=0): - """round method for round halfs to nearest even algorithm aka banker's rounding - compatible with python3""" +def rounded(num, precision=0, rounding_method=None): + """Round according to method set in system setting, defaults to banker's rounding""" precision = cint(precision) - multiplier = 10**precision + rounding_method = ( + rounding_method or frappe.get_system_settings("rounding_method") or "Round Half Even" + ) + + if rounding_method == "Round Half Even": + return _round_half_even(num, precision) + elif rounding_method == "Rounding Half Away From Zero": + return _round_away_from_zero(num, precision) + else: + frappe.throw( + frappe._("Unknown Rounding Method: {}").format(rounding_method), + exc=frappe.InvalidRoundingMethod, + ) + + +def _round_half_even(num, precision): # avoid rounding errors + multiplier = 10**precision num = round(num * multiplier if precision else num, 8) floor_num = math.floor(num) @@ -1068,6 +1088,32 @@ def rounded(num, precision=0): return (num / multiplier) if precision else num +def _round_away_from_zero(num, precision): + if num == 0: + return 0.0 + + # Epsilon is small correctional value added to correctly round numbers which can't be + # represented in IEEE 754 representation. + + # In simplified terms, the representation optimizes for absolute errors in representation + # so if a number is not representable it might be represented by a value ever so slighly + # smaller than the value itself. This becomes a problem when breaking ties for numbers + # ending with 5 when it's represented by a smaller number. By adding a very small value + # close to what's "least count" or smallest representable difference in the scale we force + # the number to be bigger than actual value, this increases representation error but + # removes rounding error. + + # References: + # - https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html + # - https://docs.python.org/3/tutorial/floatingpoint.html#representation-error + # - https://docs.python.org/3/library/functions.html#round + # - easier to understand: https://www.youtube.com/watch?v=pQs_wx8eoQ8 + + epsilon = 2.0 ** (math.log(abs(num), 2) - 52.0) + + return round(num + math.copysign(epsilon, num), precision) + + def remainder(numerator: NumericType, denominator: NumericType, precision: int = 2) -> NumericType: precision = cint(precision) multiplier = 10**precision diff --git a/pyproject.toml b/pyproject.toml index b0205edb22..837ea4624a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -102,3 +102,4 @@ Faker = "~=13.12.1" pyngrok = "~=5.0.5" unittest-xml-reporting = "~=3.0.4" watchdog = "~=2.1.9" +hypothesis = "~=6.68.2"