From dcb5ff6eacc63cebdc03f8615b18a002cb6ad23d Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Mon, 14 Nov 2011 17:36:17 +0100 Subject: [PATCH 1/9] [IMP] res.currency,base: factor out float rounding methods in tools to allow use without currency Pending: addition of tests bzr revid: odo@openerp.com-20111114163617-kw7u8rxcie7m2hj5 --- openerp/addons/base/res/res_currency.py | 41 ++++++++++--- openerp/tools/misc.py | 79 ++++++++++++++++++++++++- 2 files changed, 111 insertions(+), 9 deletions(-) diff --git a/openerp/addons/base/res/res_currency.py b/openerp/addons/base/res/res_currency.py index 32aa387e828..c0a2bebc104 100644 --- a/openerp/addons/base/res/res_currency.py +++ b/openerp/addons/base/res/res_currency.py @@ -24,7 +24,7 @@ import netsvc from osv import fields, osv import tools -from tools.misc import currency +from tools.misc import currency, float_round, float_is_zero, float_compare from tools.translate import _ CURRENCY_DISPLAY_PATTERN = re.compile(r'(\w+)\s*(?:\((.*)\))?') @@ -127,15 +127,40 @@ class res_currency(osv.osv): return [(x['id'], tools.ustr(x['name']) + (x['symbol'] and (' (' + tools.ustr(x['symbol']) + ')') or '')) for x in reads] def round(self, cr, uid, currency, amount): - if currency.rounding == 0: - return 0.0 - else: - # /!\ First member below must be rounded to full unit! - # Do not pass a rounding digits value to round() - return round(amount / currency.rounding) * currency.rounding + """Return ``amount`` rounded according to ``currency``'s + rounding rules, also minimizing IEEE-754 floating point + representation errors. + + :param browse_record currency: currency for which we are rounding + :param float amount: the amount to round + :return: rounded float + """ + return float_round(amount, precision_rounding=currency.rounding) + + def compare_amounts(self, cr, uid, currency, amount1, amount2): + """Compare ``amount1`` and ``amount2`` according to ``currency``'s + rounding rules, and return (resp.) -1, 0 or 1, if ``amount1`` + is (resp.) lower than, equal to, or greater than ``amount2``. + + For example 1.432 and 1.431 are equal if currency is rounded to + 2 digits, so this method would return 0 + + :param browse_record currency: currency for which we are rounding + :param float amount1: first amount to compare + :param float amount2: second amount to compare + :return: (resp.) -1, 0 or 1, if ``amount1`` is (resp.) lower than, + equal to, or greater than ``amount2``, according to + ``currency``'s rounding. + """ + return float_compare(amount1, amount2, precision_rounding=currency.rounding) def is_zero(self, cr, uid, currency, amount): - return abs(self.round(cr, uid, currency, amount)) < currency.rounding + """Returns true if ``amount`` is small enough to be treated as + zero according to ``currency``'s rounding rules. + :param browse_record currency: currency for which we are rounding + :param float amount: amount to compare with currency's zero + """ + return float_is_zero(amount, precision_rounding=currency.rounding) def _get_conversion_rate(self, cr, uid, from_currency, to_currency, context=None): if context is None: diff --git a/openerp/tools/misc.py b/openerp/tools/misc.py index dbccbe5b217..ae28567e19b 100644 --- a/openerp/tools/misc.py +++ b/openerp/tools/misc.py @@ -1200,4 +1200,81 @@ class UnquoteEvalContext(defaultdict): def __missing__(self, key): return unquote(key) -# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: +def _float_check_precision(precision_digits=None, precision_rounding=None): + assert (precision_digits is not None or precision_rounding is not None) and \ + not (precision_digits and precision_rounding),\ + "exactly one of precision_digits and precision_rounding must be specified" + if precision_digits is not None: + return 10 ** -precision_digits + return precision_rounding + +def float_round(amount, precision_digits=None, precision_rounding=None): + """Return ``amount`` rounded to ``precision_digits`` + decimal digits, minimizing IEEE-754 floating point representation + errors. + Precision must be given by ``precision_digits`` or ``precision_rounding``, + not both! + Example on Python 2.7.2:: + + >>> round_float(2.675) + 2.68 + >>> round(2.675,2) + 2.67 + + :param float amount: the amount to round + :param int precision_digits: number of decimal digits to round to. + :param float precision_rounding: decimal number representing the minimum + non-zero value at the desired precision (for example, 0.01 for a + 2-digit precision). + :return: rounded float + """ + rounding_factor = _float_check_precision(precision_digits=precision_digits, + precision_rounding=precision_rounding) + if rounding_factor == 0: return 0.0 + # /!\ First member below must be rounded to full unit! + # Do not pass rounding digits to round()! + return round(amount / rounding_factor) * rounding_factor + +def float_is_zero(amount, precision_digits=None, precision_rounding=None): + """Returns true if ``amount`` is small enough to be treated as + zero at the given precision. + Precision must be given by ``precision_digits`` or ``precision_rounding``, + not both! + + :param int precision_digits: number of decimal digits to round to. + :param float precision_rounding: decimal number representing the minimum + non-zero value at the desired precision (for example, 0.01 for a + 2-digit precision). + :param float amount: amount to compare with currency's zero + :return: True if ``amount`` is considered 0 + """ + rounding_factor = _float_check_precision(precision_digits=precision_digits, + precision_rounding=precision_rounding) + return abs(float_round(amount, precision_rounding=rounding_factor)) < rounding_factor + +def float_compare(amount1, amount2, precision_digits=None, precision_rounding=None): + """Compare ``amount1`` and ``amount2`` according + to the given precision. + Precision must be given by ``precision_digits`` or ``precision_rounding``, + not both! + + For example 1.432 and 1.431 are equal at 2 digits precision, + so this method would return 0 + + :param int precision_digits: number of decimal digits to round to. + :param float precision_rounding: decimal number representing the minimum + non-zero value at the desired precision (for example, 0.01 for a + 2-digit precision). + :param float amount1: first amount to compare + :param float amount2: second amount to compare + :return: (resp.) -1, 0 or 1, if ``amount1`` is (resp.) lower than, + equal to, or greater than ``amount2``, at the given precision. + """ + rounding_factor = _float_check_precision(precision_digits=precision_digits, + precision_rounding=precision_rounding) + delta = amount1 - amount2 + if float_is_zero(delta, precision_rounding=rounding_factor): return 0 + delta = float_round(delta, precision_rounding=rounding_factor) + return -1 if delta < 0 else 1 + +# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: \ No newline at end of file From ffe640cecea84380fd11e9a05bb6bc4882fef852 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Mon, 14 Nov 2011 19:23:10 +0100 Subject: [PATCH 2/9] [IMP] tools.float_*: added tests and docstrings bzr revid: odo@openerp.com-20111114182310-kkzh78uej0uuwhaz --- openerp/addons/base/res/res_currency.py | 23 ++++++--- openerp/addons/base/test/base_test.yml | 68 +++++++++++++++++++++++++ openerp/tools/misc.py | 31 ++++++++--- 3 files changed, 107 insertions(+), 15 deletions(-) diff --git a/openerp/addons/base/res/res_currency.py b/openerp/addons/base/res/res_currency.py index c0a2bebc104..c67c7d65260 100644 --- a/openerp/addons/base/res/res_currency.py +++ b/openerp/addons/base/res/res_currency.py @@ -128,8 +128,7 @@ class res_currency(osv.osv): def round(self, cr, uid, currency, amount): """Return ``amount`` rounded according to ``currency``'s - rounding rules, also minimizing IEEE-754 floating point - representation errors. + rounding rules. :param browse_record currency: currency for which we are rounding :param float amount: the amount to round @@ -138,12 +137,16 @@ class res_currency(osv.osv): return float_round(amount, precision_rounding=currency.rounding) def compare_amounts(self, cr, uid, currency, amount1, amount2): - """Compare ``amount1`` and ``amount2`` according to ``currency``'s - rounding rules, and return (resp.) -1, 0 or 1, if ``amount1`` - is (resp.) lower than, equal to, or greater than ``amount2``. + """Compare ``amount1`` and ``amount2`` after rounding them according to the + given currency's precision.. + An amount is considered lower/greater than another amount if their rounded + value is different. This is not the same as having a non-zero difference! - For example 1.432 and 1.431 are equal if currency is rounded to - 2 digits, so this method would return 0 + For example 1.432 and 1.431 are equal at 2 digits precision, + so this method would return 0. + However 0.006 and 0.002 are considered different (returns 1) because + they respectively round to 0.01 and 0.0, even though + 0.006-0.002 = 0.004 which would be considered zero at 2 digits precision. :param browse_record currency: currency for which we are rounding :param float amount1: first amount to compare @@ -157,6 +160,12 @@ class res_currency(osv.osv): def is_zero(self, cr, uid, currency, amount): """Returns true if ``amount`` is small enough to be treated as zero according to ``currency``'s rounding rules. + + Warning: ``is_zero(amount1-amount2)`` is not always equivalent to + ``compare_amounts(amount1,amount2) == 0``, as the former will round after + computing the difference, while the latter will round before, giving + different results for e.g. 0.006 and 0.002 at 2 digits precision. + :param browse_record currency: currency for which we are rounding :param float amount: amount to compare with currency's zero """ diff --git a/openerp/addons/base/test/base_test.yml b/openerp/addons/base/test/base_test.yml index 6ea578a8659..9b686ccb0d3 100644 --- a/openerp/addons/base/test/base_test.yml +++ b/openerp/addons/base/test/base_test.yml @@ -144,3 +144,71 @@ !python {model: res.partner.category}: | self.pool._init = True +- + "Float precision tests: verify that float rounding methods are working correctly" +- + !python {model: res.currency}: | + currency = self.browse(cr, uid, ref('base.EUR')) + def try_round(self, cr, currency, amount, expected): + result = str(self.round(cr, 1, currency, amount)) + assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) + try_round(self, cr, currency, 2.674,'2.67') + try_round(self, cr, currency, 2.675,'2.68') # in Python 2.7.2, round(2.675,2) gives 2.67 + try_round(self, cr, currency, 0.001,'0.0') + try_round(self, cr, currency, 0.0049,'0.0') # 0.0049 is closer to 0 than to 0.01, so should round down + try_round(self, cr, currency, 0.005,'0.01') # the rule is to round half up + + def try_zero(self, cr, currency, amount, expected): + assert self.is_zero(cr, 1, currency, amount) == expected, "Rounding error: %s should be zero!" % amount + try_zero(self, cr, currency, 0.01, False) + try_zero(self, cr, currency, 0.001, True) + try_zero(self, cr, currency, 0.0046, True) + try_zero(self, cr, currency, 2.68-2.675, False) # 2.68 - 2.675 = 0.005 -> rounds to 0.01 + try_zero(self, cr, currency, 2.68-2.676, True) # 2.68 - 2.675 = 0.004 -> rounds to 0.0 + + def try_compare(self, cr, currency, amount1, amount2, expected): + assert self.compare_amounts(cr, 1, currency, amount1, amount2) == expected, \ + "Rounding error, compare_amounts(%s,%s) should be %s" % (amount1, amount2, expected) + try_compare(self, cr, currency, 0.001, 0.001, 0) + try_compare(self, cr, currency, 0.001, 0.002, 0) + try_compare(self, cr, currency, 2.675, 2.68, 0) + try_compare(self, cr, currency, 2.676, 2.68, 0) + try_compare(self, cr, currency, 2.674, 2.68, -1) + try_compare(self, cr, currency, 3, 2.68, 1) + try_compare(self, cr, currency, 0.01, 0, 1) + + from tools import float_compare, float_is_zero, float_round + def try_round_digits(float_round, amount, expected): + result = str(float_round(amount, precision_digits=3)) + assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) + try_round_digits(float_round, 2.6745, '2.675') + try_round_digits(float_round, 2.6744, '2.674') + try_round_digits(float_round, 0.0004, '0.0') + + def try_zero_digits(float_is_zero, amount, expected): + assert float_is_zero(amount, precision_digits=3) == expected, "Rounding error: %s should be zero!" % amount + try_zero_digits(float_is_zero, 0.0002, True) + try_zero_digits(float_is_zero, 0.00034, True) + try_zero_digits(float_is_zero, 0.0005, False) + try_zero_digits(float_is_zero, 0.0008, False) + + def try_compare_digits(float_compare, amount1, amount2, expected): + assert float_compare(amount1, amount2, precision_digits=3) == expected, \ + "Rounding error, compare_amounts(%s,%s) should be %s" % (amount1, amount2, expected) + try_compare_digits(float_compare, 0.0003, 0.0004, 0) + try_compare_digits(float_compare, 0.0002, 0.0005, -1) + try_compare_digits(float_compare, 0.0009, 0.0004, 1) + + # specifying 2 precisions is illegal: + try: + float_is_zero(0.01, precision_digits=3, precision_rounding=0.01) + except AssertionError: + pass + try: + float_compare(0.01, 0.02, precision_digits=3, precision_rounding=0.01) + except AssertionError: + pass + try: + float_round(0.01, precision_digits=3, precision_rounding=0.01) + except AssertionError: + pass diff --git a/openerp/tools/misc.py b/openerp/tools/misc.py index ae28567e19b..496767f7c41 100644 --- a/openerp/tools/misc.py +++ b/openerp/tools/misc.py @@ -1210,11 +1210,13 @@ def _float_check_precision(precision_digits=None, precision_rounding=None): def float_round(amount, precision_digits=None, precision_rounding=None): """Return ``amount`` rounded to ``precision_digits`` - decimal digits, minimizing IEEE-754 floating point representation + decimal digits, minimizing IEEE-854 floating point representation errors. Precision must be given by ``precision_digits`` or ``precision_rounding``, not both! - Example on Python 2.7.2:: + + To illustrate how this is different from the default round() builtin, + here is an example (depends on Python version, here is for v2.7.2 x64):: >>> round_float(2.675) 2.68 @@ -1241,6 +1243,11 @@ def float_is_zero(amount, precision_digits=None, precision_rounding=None): Precision must be given by ``precision_digits`` or ``precision_rounding``, not both! + Warning: ``float_is_zero(amount1-amount2)`` is not always equivalent to + ``float_compare(amount1,amount2) == 0``, as the former will round after + computing the difference, while the latter will round before, giving + different results for e.g. 0.006 and 0.002 at 2 digits precision. + :param int precision_digits: number of decimal digits to round to. :param float precision_rounding: decimal number representing the minimum non-zero value at the desired precision (for example, 0.01 for a @@ -1253,13 +1260,20 @@ def float_is_zero(amount, precision_digits=None, precision_rounding=None): return abs(float_round(amount, precision_rounding=rounding_factor)) < rounding_factor def float_compare(amount1, amount2, precision_digits=None, precision_rounding=None): - """Compare ``amount1`` and ``amount2`` according - to the given precision. - Precision must be given by ``precision_digits`` or ``precision_rounding``, - not both! + """Compare ``amount1`` and ``amount2`` after rounding them according to the + given precision. An amount is considered lower/greater than another amount + if their rounded value is different. This is not the same as having a + non-zero difference! For example 1.432 and 1.431 are equal at 2 digits precision, so this method would return 0 + However 0.006 and 0.002 are considered different (returns 1) because + they respectively round to 0.01 and 0.0, even though + 0.006-0.002 = 0.004 which would be considered zero at 2 digits precision. + + + Precision must be given by ``precision_digits`` or ``precision_rounding``, + not both! :param int precision_digits: number of decimal digits to round to. :param float precision_rounding: decimal number representing the minimum @@ -1272,9 +1286,10 @@ def float_compare(amount1, amount2, precision_digits=None, precision_rounding=No """ rounding_factor = _float_check_precision(precision_digits=precision_digits, precision_rounding=precision_rounding) + amount1 = float_round(amount1, precision_rounding=rounding_factor) + amount2 = float_round(amount2, precision_rounding=rounding_factor) delta = amount1 - amount2 if float_is_zero(delta, precision_rounding=rounding_factor): return 0 - delta = float_round(delta, precision_rounding=rounding_factor) - return -1 if delta < 0 else 1 + return -1 if delta < 0.0 else 1 # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: \ No newline at end of file From ba2b2bdae168ef5d5451798925bfcb5df1a08d7b Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Tue, 15 Nov 2011 12:36:05 +0100 Subject: [PATCH 3/9] [IMP] tools.float_*: more tests with negative values, some cleanup bzr revid: odo@openerp.com-20111115113605-l5e6gg675qbrvjz6 --- openerp/addons/base/test/base_test.yml | 122 +++++++++++++++++-------- openerp/tools/misc.py | 48 +++++----- 2 files changed, 110 insertions(+), 60 deletions(-) diff --git a/openerp/addons/base/test/base_test.yml b/openerp/addons/base/test/base_test.yml index 9b686ccb0d3..56d723bf8a3 100644 --- a/openerp/addons/base/test/base_test.yml +++ b/openerp/addons/base/test/base_test.yml @@ -145,61 +145,111 @@ self.pool._init = True - - "Float precision tests: verify that float rounding methods are working correctly" + "Float precision tests: verify that float rounding methods are working correctly via res.currency" - !python {model: res.currency}: | currency = self.browse(cr, uid, ref('base.EUR')) - def try_round(self, cr, currency, amount, expected): + def try_round(amount, expected, self=self, cr=cr, currency=currency): result = str(self.round(cr, 1, currency, amount)) assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) - try_round(self, cr, currency, 2.674,'2.67') - try_round(self, cr, currency, 2.675,'2.68') # in Python 2.7.2, round(2.675,2) gives 2.67 - try_round(self, cr, currency, 0.001,'0.0') - try_round(self, cr, currency, 0.0049,'0.0') # 0.0049 is closer to 0 than to 0.01, so should round down - try_round(self, cr, currency, 0.005,'0.01') # the rule is to round half up + try_round(2.674,'2.67') + try_round(2.675,'2.68') # in Python 2.7.2, round(2.675,2) gives 2.67 + try_round(-2.675,'-2.68') # in Python 2.7.2, round(2.675,2) gives 2.67 + try_round(0.001,'0.0') + try_round(-0.001,'-0.0') + try_round(0.0049,'0.0') # 0.0049 is closer to 0 than to 0.01, so should round down + try_round(0.005,'0.01') # the rule is to round half away from zero + try_round(-0.005,'-0.01') # the rule is to round half away from zero - def try_zero(self, cr, currency, amount, expected): + def try_zero(amount, expected, self=self, cr=cr, currency=currency): assert self.is_zero(cr, 1, currency, amount) == expected, "Rounding error: %s should be zero!" % amount - try_zero(self, cr, currency, 0.01, False) - try_zero(self, cr, currency, 0.001, True) - try_zero(self, cr, currency, 0.0046, True) - try_zero(self, cr, currency, 2.68-2.675, False) # 2.68 - 2.675 = 0.005 -> rounds to 0.01 - try_zero(self, cr, currency, 2.68-2.676, True) # 2.68 - 2.675 = 0.004 -> rounds to 0.0 + try_zero(0.01, False) + try_zero(-0.01, False) + try_zero(0.001, True) + try_zero(-0.001, True) + try_zero(0.0046, True) + try_zero(-0.0046, True) + try_zero(2.68-2.675, False) # 2.68 - 2.675 = 0.005 -> rounds to 0.01 + try_zero(2.68-2.676, True) # 2.68 - 2.675 = 0.004 -> rounds to 0.0 + try_zero(2.676-2.68, True) # 2.675 - 2.68 = -0.004 -> rounds to -0.0 + try_zero(2.675-2.68, False) # 2.675 - 2.68 = -0.005 -> rounds to -0.01 - def try_compare(self, cr, currency, amount1, amount2, expected): + def try_compare(amount1, amount2, expected, self=self, cr=cr, currency=currency): assert self.compare_amounts(cr, 1, currency, amount1, amount2) == expected, \ "Rounding error, compare_amounts(%s,%s) should be %s" % (amount1, amount2, expected) - try_compare(self, cr, currency, 0.001, 0.001, 0) - try_compare(self, cr, currency, 0.001, 0.002, 0) - try_compare(self, cr, currency, 2.675, 2.68, 0) - try_compare(self, cr, currency, 2.676, 2.68, 0) - try_compare(self, cr, currency, 2.674, 2.68, -1) - try_compare(self, cr, currency, 3, 2.68, 1) - try_compare(self, cr, currency, 0.01, 0, 1) + try_compare(0.001, 0.001, 0) + try_compare(-0.001, -0.001, 0) + try_compare(0.001, 0.002, 0) + try_compare(-0.001, -0.002, 0) + try_compare(2.675, 2.68, 0) + try_compare(2.676, 2.68, 0) + try_compare(-2.676, -2.68, 0) + try_compare(2.674, 2.68, -1) + try_compare(-2.674, -2.68, 1) + try_compare(3, 2.68, 1) + try_compare(-3, -2.68, -1) + try_compare(0.01, 0, 1) + try_compare(-0.01, 0, -1) +- + "Float precision tests: verify that float rounding methods are working correctly via tools" +- + !python {model: res.currency}: | from tools import float_compare, float_is_zero, float_round - def try_round_digits(float_round, amount, expected): - result = str(float_round(amount, precision_digits=3)) + def try_round(amount, expected, precision_digits=3, float_round=float_round): + result = str(float_round(amount, precision_digits=precision_digits)) assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) - try_round_digits(float_round, 2.6745, '2.675') - try_round_digits(float_round, 2.6744, '2.674') - try_round_digits(float_round, 0.0004, '0.0') + try_round(2.6745, '2.675') + try_round(-2.6745, '-2.675') + try_round(2.6744, '2.674') + try_round(-2.6744, '-2.674') + try_round(0.0004, '0.0') + try_round(-0.0004, '-0.0') + #try_round(357.4555, '357.456') + #try_round(-357.4555, '-357.456') + try_round(457.4554, '457.455') + try_round(-457.4554, '-457.455') - def try_zero_digits(float_is_zero, amount, expected): + + def try_zero(amount, expected, float_is_zero=float_is_zero): assert float_is_zero(amount, precision_digits=3) == expected, "Rounding error: %s should be zero!" % amount - try_zero_digits(float_is_zero, 0.0002, True) - try_zero_digits(float_is_zero, 0.00034, True) - try_zero_digits(float_is_zero, 0.0005, False) - try_zero_digits(float_is_zero, 0.0008, False) + try_zero(0.0002, True) + try_zero(-0.0002, True) + try_zero(0.00034, True) + try_zero(0.0005, False) + try_zero(-0.0005, False) + try_zero(0.0008, False) + try_zero(-0.0008, False) - def try_compare_digits(float_compare, amount1, amount2, expected): + def try_compare(amount1, amount2, expected, float_compare=float_compare): assert float_compare(amount1, amount2, precision_digits=3) == expected, \ "Rounding error, compare_amounts(%s,%s) should be %s" % (amount1, amount2, expected) - try_compare_digits(float_compare, 0.0003, 0.0004, 0) - try_compare_digits(float_compare, 0.0002, 0.0005, -1) - try_compare_digits(float_compare, 0.0009, 0.0004, 1) + try_compare(0.0003, 0.0004, 0) + try_compare(-0.0003, -0.0004, 0) + try_compare(0.0002, 0.0005, -1) + try_compare(-0.0002, -0.0005, 1) + try_compare(0.0009, 0.0004, 1) + try_compare(-0.0009, -0.0004, -1) + try_compare(557.4555, 557.4556, 0) + try_compare(-557.4555, -557.4556, 0) + try_compare(657.4444, 657.445, -1) + try_compare(-657.4444, -657.445, 1) - # specifying 2 precisions is illegal: + # Rounding to different precision_roundings + def try_round(amount, expected, precision_rounding=None, float_round=float_round): + result = str(float_round(amount, precision_rounding=precision_rounding)) + assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) + try_round(-457.4554, '-457.45', precision_rounding=0.05) + try_round(457.444, '457.5', precision_rounding=0.5) + try_round(457.3, '455.0', precision_rounding=5) + try_round(457.5, '460.0', precision_rounding=5) + try_round(457.1, '456.0', precision_rounding=3) + +- + "Float precision tests: verify that invalid parameters are forbidden" +- + !python {model: res.currency}: | + from tools import float_compare, float_is_zero, float_round try: float_is_zero(0.01, precision_digits=3, precision_rounding=0.01) except AssertionError: diff --git a/openerp/tools/misc.py b/openerp/tools/misc.py index 496767f7c41..5b5713749ac 100644 --- a/openerp/tools/misc.py +++ b/openerp/tools/misc.py @@ -1208,9 +1208,9 @@ def _float_check_precision(precision_digits=None, precision_rounding=None): return 10 ** -precision_digits return precision_rounding -def float_round(amount, precision_digits=None, precision_rounding=None): - """Return ``amount`` rounded to ``precision_digits`` - decimal digits, minimizing IEEE-854 floating point representation +def float_round(value, precision_digits=None, precision_rounding=None): + """Return ``value`` rounded to ``precision_digits`` + decimal digits, minimizing IEEE-754 floating point representation errors. Precision must be given by ``precision_digits`` or ``precision_rounding``, not both! @@ -1223,7 +1223,7 @@ def float_round(amount, precision_digits=None, precision_rounding=None): >>> round(2.675,2) 2.67 - :param float amount: the amount to round + :param float value: the value to round :param int precision_digits: number of decimal digits to round to. :param float precision_rounding: decimal number representing the minimum non-zero value at the desired precision (for example, 0.01 for a @@ -1233,18 +1233,18 @@ def float_round(amount, precision_digits=None, precision_rounding=None): rounding_factor = _float_check_precision(precision_digits=precision_digits, precision_rounding=precision_rounding) if rounding_factor == 0: return 0.0 - # /!\ First member below must be rounded to full unit! - # Do not pass rounding digits to round()! - return round(amount / rounding_factor) * rounding_factor -def float_is_zero(amount, precision_digits=None, precision_rounding=None): - """Returns true if ``amount`` is small enough to be treated as + # Then round to integer wrt. rounding factor + return round(value / rounding_factor) * rounding_factor + +def float_is_zero(value, precision_digits=None, precision_rounding=None): + """Returns true if ``value`` is small enough to be treated as zero at the given precision. Precision must be given by ``precision_digits`` or ``precision_rounding``, not both! - Warning: ``float_is_zero(amount1-amount2)`` is not always equivalent to - ``float_compare(amount1,amount2) == 0``, as the former will round after + Warning: ``float_is_zero(value1-value2)`` is not always equivalent to + ``float_compare(value1,value2) == 0``, as the former will round after computing the difference, while the latter will round before, giving different results for e.g. 0.006 and 0.002 at 2 digits precision. @@ -1252,16 +1252,16 @@ def float_is_zero(amount, precision_digits=None, precision_rounding=None): :param float precision_rounding: decimal number representing the minimum non-zero value at the desired precision (for example, 0.01 for a 2-digit precision). - :param float amount: amount to compare with currency's zero - :return: True if ``amount`` is considered 0 + :param float value: value to compare with currency's zero + :return: True if ``value`` is considered 0 """ rounding_factor = _float_check_precision(precision_digits=precision_digits, precision_rounding=precision_rounding) - return abs(float_round(amount, precision_rounding=rounding_factor)) < rounding_factor + return abs(float_round(value, precision_rounding=rounding_factor)) < rounding_factor -def float_compare(amount1, amount2, precision_digits=None, precision_rounding=None): - """Compare ``amount1`` and ``amount2`` after rounding them according to the - given precision. An amount is considered lower/greater than another amount +def float_compare(value1, value2, precision_digits=None, precision_rounding=None): + """Compare ``value1`` and ``value2`` after rounding them according to the + given precision. A value is considered lower/greater than another value if their rounded value is different. This is not the same as having a non-zero difference! @@ -1279,16 +1279,16 @@ def float_compare(amount1, amount2, precision_digits=None, precision_rounding=No :param float precision_rounding: decimal number representing the minimum non-zero value at the desired precision (for example, 0.01 for a 2-digit precision). - :param float amount1: first amount to compare - :param float amount2: second amount to compare - :return: (resp.) -1, 0 or 1, if ``amount1`` is (resp.) lower than, - equal to, or greater than ``amount2``, at the given precision. + :param float value1: first value to compare + :param float value2: second value to compare + :return: (resp.) -1, 0 or 1, if ``value1`` is (resp.) lower than, + equal to, or greater than ``value2``, at the given precision. """ rounding_factor = _float_check_precision(precision_digits=precision_digits, precision_rounding=precision_rounding) - amount1 = float_round(amount1, precision_rounding=rounding_factor) - amount2 = float_round(amount2, precision_rounding=rounding_factor) - delta = amount1 - amount2 + value1 = float_round(value1, precision_rounding=rounding_factor) + value2 = float_round(value2, precision_rounding=rounding_factor) + delta = value1 - value2 if float_is_zero(delta, precision_rounding=rounding_factor): return 0 return -1 if delta < 0.0 else 1 From 9ddd05fb3ae83a6517b667009d34ab1826582cb7 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Fri, 16 Dec 2011 17:04:26 +0100 Subject: [PATCH 4/9] [REF] float_utils: moved in its own python module bzr revid: odo@openerp.com-20111216160426-gya8d1d1zymcz8os --- openerp/addons/base/res/res_currency.py | 2 +- openerp/tools/__init__.py | 1 + openerp/tools/float_utils.py | 112 ++++++++++++++++++++++++ openerp/tools/misc.py | 92 ------------------- 4 files changed, 114 insertions(+), 93 deletions(-) create mode 100644 openerp/tools/float_utils.py diff --git a/openerp/addons/base/res/res_currency.py b/openerp/addons/base/res/res_currency.py index c67c7d65260..098b0179d57 100644 --- a/openerp/addons/base/res/res_currency.py +++ b/openerp/addons/base/res/res_currency.py @@ -24,7 +24,7 @@ import netsvc from osv import fields, osv import tools -from tools.misc import currency, float_round, float_is_zero, float_compare +from tools import float_round, float_is_zero, float_compare from tools.translate import _ CURRENCY_DISPLAY_PATTERN = re.compile(r'(\w+)\s*(?:\((.*)\))?') diff --git a/openerp/tools/__init__.py b/openerp/tools/__init__.py index 3d1cdc960ab..6875ebd78fc 100644 --- a/openerp/tools/__init__.py +++ b/openerp/tools/__init__.py @@ -31,6 +31,7 @@ from amount_to_text_en import * from pdf_utils import * from yaml_import import * from sql import * +from float_utils import * #.apidoc title: Tools diff --git a/openerp/tools/float_utils.py b/openerp/tools/float_utils.py new file mode 100644 index 00000000000..dde6aa5962c --- /dev/null +++ b/openerp/tools/float_utils.py @@ -0,0 +1,112 @@ +# -*- coding: utf-8 -*- +############################################################################## +# +# OpenERP, Open Source Business Applications +# Copyright (c) 2011 OpenERP S.A. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +############################################################################## + +def _float_check_precision(precision_digits=None, precision_rounding=None): + assert (precision_digits is not None or precision_rounding is not None) and \ + not (precision_digits and precision_rounding),\ + "exactly one of precision_digits and precision_rounding must be specified" + if precision_digits is not None: + return 10 ** -precision_digits + return precision_rounding + +def float_round(value, precision_digits=None, precision_rounding=None): + """Return ``value`` rounded to ``precision_digits`` + decimal digits, minimizing IEEE-754 floating point representation + errors. + Precision must be given by ``precision_digits`` or ``precision_rounding``, + not both! + + To illustrate how this is different from the default round() builtin, + here is an example (depends on Python version, here is for v2.7.2 x64):: + + >>> round_float(2.675) + 2.68 + >>> round(2.675,2) + 2.67 + + :param float value: the value to round + :param int precision_digits: number of decimal digits to round to. + :param float precision_rounding: decimal number representing the minimum + non-zero value at the desired precision (for example, 0.01 for a + 2-digit precision). + :return: rounded float + """ + rounding_factor = _float_check_precision(precision_digits=precision_digits, + precision_rounding=precision_rounding) + if rounding_factor == 0: return 0.0 + + # Then round to integer wrt. rounding factor + return round(value / rounding_factor) * rounding_factor + +def float_is_zero(value, precision_digits=None, precision_rounding=None): + """Returns true if ``value`` is small enough to be treated as + zero at the given precision. + Precision must be given by ``precision_digits`` or ``precision_rounding``, + not both! + + Warning: ``float_is_zero(value1-value2)`` is not always equivalent to + ``float_compare(value1,value2) == 0``, as the former will round after + computing the difference, while the latter will round before, giving + different results for e.g. 0.006 and 0.002 at 2 digits precision. + + :param int precision_digits: number of decimal digits to round to. + :param float precision_rounding: decimal number representing the minimum + non-zero value at the desired precision (for example, 0.01 for a + 2-digit precision). + :param float value: value to compare with currency's zero + :return: True if ``value`` is considered 0 + """ + rounding_factor = _float_check_precision(precision_digits=precision_digits, + precision_rounding=precision_rounding) + return abs(float_round(value, precision_rounding=rounding_factor)) < rounding_factor + +def float_compare(value1, value2, precision_digits=None, precision_rounding=None): + """Compare ``value1`` and ``value2`` after rounding them according to the + given precision. A value is considered lower/greater than another value + if their rounded value is different. This is not the same as having a + non-zero difference! + + For example 1.432 and 1.431 are equal at 2 digits precision, + so this method would return 0 + However 0.006 and 0.002 are considered different (returns 1) because + they respectively round to 0.01 and 0.0, even though + 0.006-0.002 = 0.004 which would be considered zero at 2 digits precision. + + + Precision must be given by ``precision_digits`` or ``precision_rounding``, + not both! + + :param int precision_digits: number of decimal digits to round to. + :param float precision_rounding: decimal number representing the minimum + non-zero value at the desired precision (for example, 0.01 for a + 2-digit precision). + :param float value1: first value to compare + :param float value2: second value to compare + :return: (resp.) -1, 0 or 1, if ``value1`` is (resp.) lower than, + equal to, or greater than ``value2``, at the given precision. + """ + rounding_factor = _float_check_precision(precision_digits=precision_digits, + precision_rounding=precision_rounding) + value1 = float_round(value1, precision_rounding=rounding_factor) + value2 = float_round(value2, precision_rounding=rounding_factor) + delta = value1 - value2 + if float_is_zero(delta, precision_rounding=rounding_factor): return 0 + return -1 if delta < 0.0 else 1 \ No newline at end of file diff --git a/openerp/tools/misc.py b/openerp/tools/misc.py index 5b5713749ac..7fdb2f07386 100644 --- a/openerp/tools/misc.py +++ b/openerp/tools/misc.py @@ -1200,96 +1200,4 @@ class UnquoteEvalContext(defaultdict): def __missing__(self, key): return unquote(key) -def _float_check_precision(precision_digits=None, precision_rounding=None): - assert (precision_digits is not None or precision_rounding is not None) and \ - not (precision_digits and precision_rounding),\ - "exactly one of precision_digits and precision_rounding must be specified" - if precision_digits is not None: - return 10 ** -precision_digits - return precision_rounding - -def float_round(value, precision_digits=None, precision_rounding=None): - """Return ``value`` rounded to ``precision_digits`` - decimal digits, minimizing IEEE-754 floating point representation - errors. - Precision must be given by ``precision_digits`` or ``precision_rounding``, - not both! - - To illustrate how this is different from the default round() builtin, - here is an example (depends on Python version, here is for v2.7.2 x64):: - - >>> round_float(2.675) - 2.68 - >>> round(2.675,2) - 2.67 - - :param float value: the value to round - :param int precision_digits: number of decimal digits to round to. - :param float precision_rounding: decimal number representing the minimum - non-zero value at the desired precision (for example, 0.01 for a - 2-digit precision). - :return: rounded float - """ - rounding_factor = _float_check_precision(precision_digits=precision_digits, - precision_rounding=precision_rounding) - if rounding_factor == 0: return 0.0 - - # Then round to integer wrt. rounding factor - return round(value / rounding_factor) * rounding_factor - -def float_is_zero(value, precision_digits=None, precision_rounding=None): - """Returns true if ``value`` is small enough to be treated as - zero at the given precision. - Precision must be given by ``precision_digits`` or ``precision_rounding``, - not both! - - Warning: ``float_is_zero(value1-value2)`` is not always equivalent to - ``float_compare(value1,value2) == 0``, as the former will round after - computing the difference, while the latter will round before, giving - different results for e.g. 0.006 and 0.002 at 2 digits precision. - - :param int precision_digits: number of decimal digits to round to. - :param float precision_rounding: decimal number representing the minimum - non-zero value at the desired precision (for example, 0.01 for a - 2-digit precision). - :param float value: value to compare with currency's zero - :return: True if ``value`` is considered 0 - """ - rounding_factor = _float_check_precision(precision_digits=precision_digits, - precision_rounding=precision_rounding) - return abs(float_round(value, precision_rounding=rounding_factor)) < rounding_factor - -def float_compare(value1, value2, precision_digits=None, precision_rounding=None): - """Compare ``value1`` and ``value2`` after rounding them according to the - given precision. A value is considered lower/greater than another value - if their rounded value is different. This is not the same as having a - non-zero difference! - - For example 1.432 and 1.431 are equal at 2 digits precision, - so this method would return 0 - However 0.006 and 0.002 are considered different (returns 1) because - they respectively round to 0.01 and 0.0, even though - 0.006-0.002 = 0.004 which would be considered zero at 2 digits precision. - - - Precision must be given by ``precision_digits`` or ``precision_rounding``, - not both! - - :param int precision_digits: number of decimal digits to round to. - :param float precision_rounding: decimal number representing the minimum - non-zero value at the desired precision (for example, 0.01 for a - 2-digit precision). - :param float value1: first value to compare - :param float value2: second value to compare - :return: (resp.) -1, 0 or 1, if ``value1`` is (resp.) lower than, - equal to, or greater than ``value2``, at the given precision. - """ - rounding_factor = _float_check_precision(precision_digits=precision_digits, - precision_rounding=precision_rounding) - value1 = float_round(value1, precision_rounding=rounding_factor) - value2 = float_round(value2, precision_rounding=rounding_factor) - delta = value1 - value2 - if float_is_zero(delta, precision_rounding=rounding_factor): return 0 - return -1 if delta < 0.0 else 1 - # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: \ No newline at end of file From dcd1c1daff6e27b26c76cb222fc8ca05b98aaae4 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Mon, 19 Dec 2011 11:57:18 +0100 Subject: [PATCH 5/9] [IMP] float_utils: better variable names, explain /epsilon/ usage bzr revid: odo@openerp.com-20111219105718-0jbz01rirl0s64ca --- openerp/tools/float_utils.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/openerp/tools/float_utils.py b/openerp/tools/float_utils.py index dde6aa5962c..effd473e269 100644 --- a/openerp/tools/float_utils.py +++ b/openerp/tools/float_utils.py @@ -58,9 +58,11 @@ def float_round(value, precision_digits=None, precision_rounding=None): def float_is_zero(value, precision_digits=None, precision_rounding=None): """Returns true if ``value`` is small enough to be treated as - zero at the given precision. + zero at the given precision (smaller than the given *epsilon*). Precision must be given by ``precision_digits`` or ``precision_rounding``, - not both! + not both! Here the precision (``10**-precision_digits`` or + ``precision_rounding``) is used as the zero *epsilon*: values smaller + than that are considered to be zero. Warning: ``float_is_zero(value1-value2)`` is not always equivalent to ``float_compare(value1,value2) == 0``, as the former will round after @@ -74,26 +76,29 @@ def float_is_zero(value, precision_digits=None, precision_rounding=None): :param float value: value to compare with currency's zero :return: True if ``value`` is considered 0 """ - rounding_factor = _float_check_precision(precision_digits=precision_digits, + epsilon = _float_check_precision(precision_digits=precision_digits, precision_rounding=precision_rounding) - return abs(float_round(value, precision_rounding=rounding_factor)) < rounding_factor + return abs(float_round(value, precision_rounding=epsilon)) < epsilon def float_compare(value1, value2, precision_digits=None, precision_rounding=None): """Compare ``value1`` and ``value2`` after rounding them according to the given precision. A value is considered lower/greater than another value if their rounded value is different. This is not the same as having a non-zero difference! - - For example 1.432 and 1.431 are equal at 2 digits precision, - so this method would return 0 - However 0.006 and 0.002 are considered different (returns 1) because - they respectively round to 0.01 and 0.0, even though - 0.006-0.002 = 0.004 which would be considered zero at 2 digits precision. - - Precision must be given by ``precision_digits`` or ``precision_rounding``, not both! + Example: 1.432 and 1.431 are equal at 2 digits precision, + so this method would return 0 + However 0.006 and 0.002 are considered different (method returns 1) because + they respectively round to 0.01 and 0.0, even though 0.006-0.002 = 0.004 + which would be considered zero at 2 digits precision. + + Warning: ``float_is_zero(value1-value2)`` is not always equivalent to + ``float_compare(value1,value2) == 0``, as the former will round after + computing the difference, while the latter will round before, giving + different results for e.g. 0.006 and 0.002 at 2 digits precision. + :param int precision_digits: number of decimal digits to round to. :param float precision_rounding: decimal number representing the minimum non-zero value at the desired precision (for example, 0.01 for a From ed53ef476137c707245e3ef50ab800f31de58aeb Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Tue, 20 Dec 2011 17:34:20 +0100 Subject: [PATCH 6/9] [IMP] float_utils: fix HALF_UP rounding according to discussions on bug 882036 Also improved test and added warning logging when crossing the precision limit due to too many significant digits. There is apparently a limitation in Python's float implementation for this. bzr revid: odo@openerp.com-20111220163420-lz0sh1h0yjkh6jdc --- openerp/addons/base/test/base_test.yml | 21 +++++- openerp/tools/float_utils.py | 89 ++++++++++++++++++++++++-- 2 files changed, 103 insertions(+), 7 deletions(-) diff --git a/openerp/addons/base/test/base_test.yml b/openerp/addons/base/test/base_test.yml index 56d723bf8a3..41c5c8d84e8 100644 --- a/openerp/addons/base/test/base_test.yml +++ b/openerp/addons/base/test/base_test.yml @@ -205,11 +205,28 @@ try_round(-2.6744, '-2.674') try_round(0.0004, '0.0') try_round(-0.0004, '-0.0') - #try_round(357.4555, '357.456') - #try_round(-357.4555, '-357.456') + try_round(357.4555, '357.456') + try_round(-357.4555, '-357.456') try_round(457.4554, '457.455') try_round(-457.4554, '-457.455') + # Extended float range test, inspired by Cloves Almeida's test on bug #882036. + fractions = [.0, .015, .01499, .675, .67499, .4555, .4555, .45555] + expecteds = ['.0', '.02', '.01', '.68', '.67', '.46', '.456', '.4556'] + precisions = [2, 2, 2, 2, 2, 2, 3, 4] + # We can't go higher than 12 significant digits with a fractional part, so + # that means max magnitude = 5 with precision 4 if sample values (x) go up to 10000 + # because that gives 5 + 4 + 4 = 13 significant digits. + for magnitude in range(5): + for i in xrange(len(fractions)): + frac, exp, prec = fractions[i], expecteds[i], precisions[i] + for sign in [-1,1]: + for x in xrange(0,10000,34): + n = x * 10**magnitude + f = sign * (n + frac) + f_exp = ('-' if f != 0 and sign == -1 else '') + str(n) + exp + try_round(f, f_exp, precision_digits=prec) + def try_zero(amount, expected, float_is_zero=float_is_zero): assert float_is_zero(amount, precision_digits=3) == expected, "Rounding error: %s should be zero!" % amount diff --git a/openerp/tools/float_utils.py b/openerp/tools/float_utils.py index effd473e269..202de89d0cf 100644 --- a/openerp/tools/float_utils.py +++ b/openerp/tools/float_utils.py @@ -19,6 +19,16 @@ # ############################################################################## +import logging +import math +from decimal import Decimal, ROUND_HALF_UP + +# When a number crosses this threshold, significant decimal +# digits may be lost when trying to render the float value, due to +# Python's float implementation. +# e.g. str(10060000.45556) == '10060000.4556' => lost 1 digit! +SIGNIFICANT_DIGITS_SCALE_LIMIT = math.log(10**12, 2) # 10**12 ~= 2**39.86 + def _float_check_precision(precision_digits=None, precision_rounding=None): assert (precision_digits is not None or precision_rounding is not None) and \ not (precision_digits and precision_rounding),\ @@ -51,10 +61,39 @@ def float_round(value, precision_digits=None, precision_rounding=None): """ rounding_factor = _float_check_precision(precision_digits=precision_digits, precision_rounding=precision_rounding) - if rounding_factor == 0: return 0.0 - - # Then round to integer wrt. rounding factor - return round(value / rounding_factor) * rounding_factor + if rounding_factor == 0 or value == 0: return 0.0 + # scale up by rounding_factor, in order to implement rounding to arbitrary + # `units` or 'rounding_factors'. + # Example: if rounding_factor is 0.5, 1.3 should round to 1.5 + # So we'll do this: scaled_value = 1.3 / 0.5 = 2.6 + # int_rounded = round(2.6) = 3 + # result = 3 * 0.5 = 1.5 + # Also, .5 is a binary fraction, so this automatically solves some tricky + # cases when rounding_factor is a negative power of 10. E.g 2.6745 is + # difficult to round to 0.001 because it does not have an exact IEEE754 + # representation, but 2674.5 is simple to round to 2675 because both + # are exactly represented. + scaled_value = value / rounding_factor + # Despite the advantage of rounding to .5 binary fractions, we still need + # to add a small epsilon value to take care of cases where the float repr + # is slightly too far below .5 to properly round *up* automatically. + # That epsilon needs to be scaled according to the order of magnitude of + # the value. (Credit: discussed with several community members on bug 882036) + epsilon_scale = math.log(abs(scaled_value), 2) + frac_part, _ = math.modf(scaled_value) + if frac_part and epsilon_scale > SIGNIFICANT_DIGITS_SCALE_LIMIT: + print 'Float rounding of %r to %r precision requires too many '\ + 'significant digits, a loss of precision may occur in the '\ + 'least significant digits' % (value,rounding_factor) + logging.getLogger('float_utils')\ + .warning('Float rounding of %r to %r precision requires too many ' + 'significant digits, a loss of precision may occur in the ' + 'least significant digits', value, rounding_factor) + epsilon = 2**(epsilon_scale-50) + scaled_value += cmp(scaled_value,0) * epsilon + rounded_value = round(scaled_value) + result = rounded_value * rounding_factor + return result def float_is_zero(value, precision_digits=None, precision_rounding=None): """Returns true if ``value`` is small enough to be treated as @@ -114,4 +153,44 @@ def float_compare(value1, value2, precision_digits=None, precision_rounding=None value2 = float_round(value2, precision_rounding=rounding_factor) delta = value1 - value2 if float_is_zero(delta, precision_rounding=rounding_factor): return 0 - return -1 if delta < 0.0 else 1 \ No newline at end of file + return -1 if delta < 0.0 else 1 + + + + + +if __name__ == "__main__": + + import time + start = time.time() + count = 0 + errors = 0 + + def try_round(amount, expected, precision_digits=3): + global count, errors; count += 1 + result = float_round(amount, precision_digits=precision_digits) + if str(result) != expected: + errors += 1 + print '###!!! Rounding error: got %s or %s, expected %s' % (str(result), repr(result), expected) + + # Extended float range test, inspired by Cloves Almeida's test on bug #882036. + fractions = [.0, .015, .01499, .675, .67499, .4555, .4555, .45555] + expecteds = ['.0', '.02', '.01', '.68', '.67', '.46', '.456', '.4556'] + precisions = [2, 2, 2, 2, 2, 2, 3, 4] + for magnitude in range(5): + for i in xrange(len(fractions)): + frac, exp, prec = fractions[i], expecteds[i], precisions[i] + for sign in [-1,1]: + for x in xrange(0,10000,17): + n = x * 10**magnitude + f = sign * (n + frac) + f_exp = ('-' if f != 0 and sign == -1 else '') + str(n) + exp + try_round(f, f_exp, precision_digits=prec) + + stop = time.time() + + # Micro-bench results: + # 47130 round calls in 0.422306060791 secs, with Python 2.6.7 on Core i3 x64 + # with decimal: + # 47130 round calls in 6.612248100021 secs, with Python 2.6.7 on Core i3 x64 + print count, " round calls, ", errors, "errors, done in ", (stop-start), 'secs' From 707bf0d8aa55a4c493d1a94393f38a1799233b96 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Tue, 20 Dec 2011 18:03:28 +0100 Subject: [PATCH 7/9] [FIX] fields.float: properly round when coercing to desired float precision lp bug: https://launchpad.net/bugs/882036 fixed bzr revid: odo@openerp.com-20111220170328-nvutckcxnitkcq9d --- openerp/osv/fields.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/openerp/osv/fields.py b/openerp/osv/fields.py index 42bc9b75c1d..0579677fd5e 100644 --- a/openerp/osv/fields.py +++ b/openerp/osv/fields.py @@ -45,6 +45,7 @@ import openerp import openerp.netsvc as netsvc import openerp.tools as tools from openerp.tools.translate import _ +from openerp.tools import float_round def _symbol_set(symb): if symb == None or symb == False: @@ -229,17 +230,17 @@ class float(_column): def __init__(self, string='unknown', digits=None, digits_compute=None, required=False, **args): _column.__init__(self, string=string, required=required, **args) self.digits = digits + # synopsis: digits_compute(cr) -> (precision, scale) self.digits_compute = digits_compute if required: warnings.warn("Making a float field `required` has no effect, as NULL values are " "automatically turned into 0.0", PendingDeprecationWarning, stacklevel=2) - def digits_change(self, cr): if self.digits_compute: - t = self.digits_compute(cr) - self._symbol_set=('%s', lambda x: ('%.'+str(t[1])+'f') % (__builtin__.float(x or 0.0),)) - self.digits = t + self.digits = self.digits_compute(cr) + precision, scale = self.digits + self._symbol_set = ('%s', lambda x: str(float_round(__builtin__.float(x or 0.0), precision_digits=scale))) class date(_column): _type = 'date' @@ -991,10 +992,9 @@ class function(_column): def digits_change(self, cr): if self.digits_compute: - t = self.digits_compute(cr) - self._symbol_set=('%s', lambda x: ('%.'+str(t[1])+'f') % (__builtin__.float(x or 0.0),)) - self.digits = t - + self.digits = self.digits_compute(cr) + precision, scale = self.digits + self._symbol_set = ('%s', lambda x: str(float_round(__builtin__.float(x or 0.0), precision_digits=scale))) def search(self, cr, uid, obj, name, args, context=None): if not self._fnct_search: From 759ccd984542ce4077dfcc9fa14198b60719dded Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Wed, 21 Dec 2011 00:47:40 +0100 Subject: [PATCH 8/9] [IMP] float_utils: simplified code, added float_repr lp bug: https://launchpad.net/bugs/882036 fixed bzr revid: odo@openerp.com-20111220234740-kotcgoz3opcbkx4b --- openerp/addons/base/test/base_test.yml | 51 +++++----- openerp/osv/fields.py | 8 +- openerp/tools/float_utils.py | 128 ++++++++++++------------- 3 files changed, 92 insertions(+), 95 deletions(-) diff --git a/openerp/addons/base/test/base_test.yml b/openerp/addons/base/test/base_test.yml index 41c5c8d84e8..fcead834d93 100644 --- a/openerp/addons/base/test/base_test.yml +++ b/openerp/addons/base/test/base_test.yml @@ -148,16 +148,20 @@ "Float precision tests: verify that float rounding methods are working correctly via res.currency" - !python {model: res.currency}: | + from tools import float_repr + from math import log10 currency = self.browse(cr, uid, ref('base.EUR')) - def try_round(amount, expected, self=self, cr=cr, currency=currency): - result = str(self.round(cr, 1, currency, amount)) + def try_round(amount, expected, self=self, cr=cr, currency=currency, float_repr=float_repr, + log10=log10): + digits = max(0,-int(log10(currency.rounding))) + result = float_repr(self.round(cr, 1, currency, amount), precision_digits=digits) assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) try_round(2.674,'2.67') try_round(2.675,'2.68') # in Python 2.7.2, round(2.675,2) gives 2.67 try_round(-2.675,'-2.68') # in Python 2.7.2, round(2.675,2) gives 2.67 - try_round(0.001,'0.0') - try_round(-0.001,'-0.0') - try_round(0.0049,'0.0') # 0.0049 is closer to 0 than to 0.01, so should round down + try_round(0.001,'0.00') + try_round(-0.001,'-0.00') + try_round(0.0049,'0.00') # 0.0049 is closer to 0 than to 0.01, so should round down try_round(0.005,'0.01') # the rule is to round half away from zero try_round(-0.005,'-0.01') # the rule is to round half away from zero @@ -195,16 +199,17 @@ "Float precision tests: verify that float rounding methods are working correctly via tools" - !python {model: res.currency}: | - from tools import float_compare, float_is_zero, float_round - def try_round(amount, expected, precision_digits=3, float_round=float_round): - result = str(float_round(amount, precision_digits=precision_digits)) + from tools import float_compare, float_is_zero, float_round, float_repr + def try_round(amount, expected, precision_digits=3, float_round=float_round, float_repr=float_repr): + result = float_repr(float_round(amount, precision_digits=precision_digits), + precision_digits=precision_digits) assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) try_round(2.6745, '2.675') try_round(-2.6745, '-2.675') try_round(2.6744, '2.674') try_round(-2.6744, '-2.674') - try_round(0.0004, '0.0') - try_round(-0.0004, '-0.0') + try_round(0.0004, '0.000') + try_round(-0.0004, '-0.000') try_round(357.4555, '357.456') try_round(-357.4555, '-357.456') try_round(457.4554, '457.455') @@ -212,16 +217,15 @@ # Extended float range test, inspired by Cloves Almeida's test on bug #882036. fractions = [.0, .015, .01499, .675, .67499, .4555, .4555, .45555] - expecteds = ['.0', '.02', '.01', '.68', '.67', '.46', '.456', '.4556'] + expecteds = ['.00', '.02', '.01', '.68', '.67', '.46', '.456', '.4556'] precisions = [2, 2, 2, 2, 2, 2, 3, 4] - # We can't go higher than 12 significant digits with a fractional part, so - # that means max magnitude = 5 with precision 4 if sample values (x) go up to 10000 - # because that gives 5 + 4 + 4 = 13 significant digits. - for magnitude in range(5): + # Note: max precision for double floats is 53 bits of precision or + # 17 significant decimal digits + for magnitude in range(7): for i in xrange(len(fractions)): frac, exp, prec = fractions[i], expecteds[i], precisions[i] for sign in [-1,1]: - for x in xrange(0,10000,34): + for x in xrange(0,10000,97): n = x * 10**magnitude f = sign * (n + frac) f_exp = ('-' if f != 0 and sign == -1 else '') + str(n) + exp @@ -252,15 +256,16 @@ try_compare(657.4444, 657.445, -1) try_compare(-657.4444, -657.445, 1) - # Rounding to different precision_roundings - def try_round(amount, expected, precision_rounding=None, float_round=float_round): - result = str(float_round(amount, precision_rounding=precision_rounding)) + # Rounding to unusual rounding units (e.g. coin values) + def try_round(amount, expected, precision_rounding=None, float_round=float_round, float_repr=float_repr): + result = float_repr(float_round(amount, precision_rounding=precision_rounding), + precision_digits=2) assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) try_round(-457.4554, '-457.45', precision_rounding=0.05) - try_round(457.444, '457.5', precision_rounding=0.5) - try_round(457.3, '455.0', precision_rounding=5) - try_round(457.5, '460.0', precision_rounding=5) - try_round(457.1, '456.0', precision_rounding=3) + try_round(457.444, '457.50', precision_rounding=0.5) + try_round(457.3, '455.00', precision_rounding=5) + try_round(457.5, '460.00', precision_rounding=5) + try_round(457.1, '456.00', precision_rounding=3) - "Float precision tests: verify that invalid parameters are forbidden" diff --git a/openerp/osv/fields.py b/openerp/osv/fields.py index 0579677fd5e..ea1b6d93329 100644 --- a/openerp/osv/fields.py +++ b/openerp/osv/fields.py @@ -45,7 +45,7 @@ import openerp import openerp.netsvc as netsvc import openerp.tools as tools from openerp.tools.translate import _ -from openerp.tools import float_round +from openerp.tools import float_round, float_repr def _symbol_set(symb): if symb == None or symb == False: @@ -240,7 +240,8 @@ class float(_column): if self.digits_compute: self.digits = self.digits_compute(cr) precision, scale = self.digits - self._symbol_set = ('%s', lambda x: str(float_round(__builtin__.float(x or 0.0), precision_digits=scale))) + self._symbol_set = ('%s', lambda x: float_repr(float_round(__builtin__.float(x or 0.0), precision_digits=scale), + precision_digits=scale)) class date(_column): _type = 'date' @@ -994,7 +995,8 @@ class function(_column): if self.digits_compute: self.digits = self.digits_compute(cr) precision, scale = self.digits - self._symbol_set = ('%s', lambda x: str(float_round(__builtin__.float(x or 0.0), precision_digits=scale))) + self._symbol_set = ('%s', lambda x: float_repr(float_round(__builtin__.float(x or 0.0), precision_digits=scale), + precision_digits=scale)) def search(self, cr, uid, obj, name, args, context=None): if not self._fnct_search: diff --git a/openerp/tools/float_utils.py b/openerp/tools/float_utils.py index 202de89d0cf..5c934114e3c 100644 --- a/openerp/tools/float_utils.py +++ b/openerp/tools/float_utils.py @@ -19,15 +19,7 @@ # ############################################################################## -import logging import math -from decimal import Decimal, ROUND_HALF_UP - -# When a number crosses this threshold, significant decimal -# digits may be lost when trying to render the float value, due to -# Python's float implementation. -# e.g. str(10060000.45556) == '10060000.4556' => lost 1 digit! -SIGNIFICANT_DIGITS_SCALE_LIMIT = math.log(10**12, 2) # 10**12 ~= 2**39.86 def _float_check_precision(precision_digits=None, precision_rounding=None): assert (precision_digits is not None or precision_rounding is not None) and \ @@ -40,20 +32,12 @@ def _float_check_precision(precision_digits=None, precision_rounding=None): def float_round(value, precision_digits=None, precision_rounding=None): """Return ``value`` rounded to ``precision_digits`` decimal digits, minimizing IEEE-754 floating point representation - errors. + errors, and applying HALF-UP (away from zero) tie-breaking rule. Precision must be given by ``precision_digits`` or ``precision_rounding``, not both! - To illustrate how this is different from the default round() builtin, - here is an example (depends on Python version, here is for v2.7.2 x64):: - - >>> round_float(2.675) - 2.68 - >>> round(2.675,2) - 2.67 - :param float value: the value to round - :param int precision_digits: number of decimal digits to round to. + :param int precision_digits: number of fractional digits to round to. :param float precision_rounding: decimal number representing the minimum non-zero value at the desired precision (for example, 0.01 for a 2-digit precision). @@ -62,58 +46,51 @@ def float_round(value, precision_digits=None, precision_rounding=None): rounding_factor = _float_check_precision(precision_digits=precision_digits, precision_rounding=precision_rounding) if rounding_factor == 0 or value == 0: return 0.0 - # scale up by rounding_factor, in order to implement rounding to arbitrary - # `units` or 'rounding_factors'. - # Example: if rounding_factor is 0.5, 1.3 should round to 1.5 - # So we'll do this: scaled_value = 1.3 / 0.5 = 2.6 - # int_rounded = round(2.6) = 3 - # result = 3 * 0.5 = 1.5 - # Also, .5 is a binary fraction, so this automatically solves some tricky - # cases when rounding_factor is a negative power of 10. E.g 2.6745 is - # difficult to round to 0.001 because it does not have an exact IEEE754 - # representation, but 2674.5 is simple to round to 2675 because both - # are exactly represented. - scaled_value = value / rounding_factor - # Despite the advantage of rounding to .5 binary fractions, we still need - # to add a small epsilon value to take care of cases where the float repr - # is slightly too far below .5 to properly round *up* automatically. - # That epsilon needs to be scaled according to the order of magnitude of - # the value. (Credit: discussed with several community members on bug 882036) - epsilon_scale = math.log(abs(scaled_value), 2) - frac_part, _ = math.modf(scaled_value) - if frac_part and epsilon_scale > SIGNIFICANT_DIGITS_SCALE_LIMIT: - print 'Float rounding of %r to %r precision requires too many '\ - 'significant digits, a loss of precision may occur in the '\ - 'least significant digits' % (value,rounding_factor) - logging.getLogger('float_utils')\ - .warning('Float rounding of %r to %r precision requires too many ' - 'significant digits, a loss of precision may occur in the ' - 'least significant digits', value, rounding_factor) - epsilon = 2**(epsilon_scale-50) - scaled_value += cmp(scaled_value,0) * epsilon - rounded_value = round(scaled_value) - result = rounded_value * rounding_factor + + # NORMALIZE - ROUND - DENORMALIZE + # In order to easily support rounding to arbitrary 'steps' (e.g. coin values), + # we normalize the value before rounding it as an integer, and de-normalize + # after rounding: e.g. float_round(1.3, precision_rounding=.5) == 1.5 + + # TIE-BREAKING: HALF-UP + # We want to apply HALF-UP tie-breaking rules, i.e. 0.5 rounds away from 0. + # Due to IEE754 float/double representation limits, the approximation of the + # real value may be slightly below the tie limit, resulting in an error of + # 1 unit in the last place (ulp) after rounding. + # For example 2.675 == 2.6749999999999998. + # To correct this, we add a very small epsilon value, scaled to the + # the order of magnitude of the value, to tip the tie-break in the right + # direction. + # Credit: discussion with OpenERP community members on bug 882036 + + normalized_value = value / rounding_factor # normalize + epsilon_magnitude = math.log(abs(normalized_value), 2) + epsilon = 2**(epsilon_magnitude-53) + normalized_value += cmp(normalized_value,0) * epsilon + rounded_value = round(normalized_value) # round to integer + result = rounded_value * rounding_factor # de-normalize return result def float_is_zero(value, precision_digits=None, precision_rounding=None): """Returns true if ``value`` is small enough to be treated as - zero at the given precision (smaller than the given *epsilon*). + zero at the given precision (smaller than the corresponding *epsilon*). + The precision (``10**-precision_digits`` or ``precision_rounding``) + is used as the zero *epsilon*: values less than that are considered + to be zero. Precision must be given by ``precision_digits`` or ``precision_rounding``, - not both! Here the precision (``10**-precision_digits`` or - ``precision_rounding``) is used as the zero *epsilon*: values smaller - than that are considered to be zero. + not both! - Warning: ``float_is_zero(value1-value2)`` is not always equivalent to + Warning: ``float_is_zero(value1-value2)`` is not equivalent to ``float_compare(value1,value2) == 0``, as the former will round after computing the difference, while the latter will round before, giving different results for e.g. 0.006 and 0.002 at 2 digits precision. - :param int precision_digits: number of decimal digits to round to. + :param int precision_digits: number of fractional digits to round to. :param float precision_rounding: decimal number representing the minimum non-zero value at the desired precision (for example, 0.01 for a 2-digit precision). - :param float value: value to compare with currency's zero - :return: True if ``value`` is considered 0 + :param float value: value to compare with the precision's zero + :return: True if ``value`` is considered zero """ epsilon = _float_check_precision(precision_digits=precision_digits, precision_rounding=precision_rounding) @@ -129,16 +106,16 @@ def float_compare(value1, value2, precision_digits=None, precision_rounding=None Example: 1.432 and 1.431 are equal at 2 digits precision, so this method would return 0 - However 0.006 and 0.002 are considered different (method returns 1) because - they respectively round to 0.01 and 0.0, even though 0.006-0.002 = 0.004 - which would be considered zero at 2 digits precision. + However 0.006 and 0.002 are considered different (this method returns 1) + because they respectively round to 0.01 and 0.0, even though + 0.006-0.002 = 0.004 which would be considered zero at 2 digits precision. - Warning: ``float_is_zero(value1-value2)`` is not always equivalent to + Warning: ``float_is_zero(value1-value2)`` is not equivalent to ``float_compare(value1,value2) == 0``, as the former will round after computing the difference, while the latter will round before, giving different results for e.g. 0.006 and 0.002 at 2 digits precision. - :param int precision_digits: number of decimal digits to round to. + :param int precision_digits: number of fractional digits to round to. :param float precision_rounding: decimal number representing the minimum non-zero value at the desired precision (for example, 0.01 for a 2-digit precision). @@ -155,8 +132,20 @@ def float_compare(value1, value2, precision_digits=None, precision_rounding=None if float_is_zero(delta, precision_rounding=rounding_factor): return 0 return -1 if delta < 0.0 else 1 +def float_repr(value, precision_digits): + """Returns a string representation of a float with the + the given number of fractional digits. This should not be + used to perform a rounding operation (this is done via + :meth:`~.float_round`), but only to produce a suitable + string representation for a float. - + :param int precision_digits: number of fractional digits to + include in the output + """ + # Can't use str() here because it seems to have an intrisic + # rounding to 12 significant digits, which causes a loss of + # precision. e.g. str(123456789.1234) == str(123456789.123)!! + return ("%%.%sf" % precision_digits) % value if __name__ == "__main__": @@ -168,20 +157,21 @@ if __name__ == "__main__": def try_round(amount, expected, precision_digits=3): global count, errors; count += 1 - result = float_round(amount, precision_digits=precision_digits) - if str(result) != expected: + result = float_repr(float_round(amount, precision_digits=precision_digits), + precision_digits=precision_digits) + if result != expected: errors += 1 - print '###!!! Rounding error: got %s or %s, expected %s' % (str(result), repr(result), expected) + print '###!!! Rounding error: got %s , expected %s' % (result, expected) # Extended float range test, inspired by Cloves Almeida's test on bug #882036. fractions = [.0, .015, .01499, .675, .67499, .4555, .4555, .45555] - expecteds = ['.0', '.02', '.01', '.68', '.67', '.46', '.456', '.4556'] + expecteds = ['.00', '.02', '.01', '.68', '.67', '.46', '.456', '.4556'] precisions = [2, 2, 2, 2, 2, 2, 3, 4] - for magnitude in range(5): + for magnitude in range(7): for i in xrange(len(fractions)): frac, exp, prec = fractions[i], expecteds[i], precisions[i] for sign in [-1,1]: - for x in xrange(0,10000,17): + for x in xrange(0,10000,97): n = x * 10**magnitude f = sign * (n + frac) f_exp = ('-' if f != 0 and sign == -1 else '') + str(n) + exp From e410b81cc4adba82e5619849da802ecb7b3e44ec Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Wed, 21 Dec 2011 02:11:00 +0100 Subject: [PATCH 9/9] [FIX] fields.float: always round floats when persisting Added test to try a full round-trip to the database as well to make sure the automatic rounding is working properly. lp bug: https://launchpad.net/bugs/882036 fixed bzr revid: odo@openerp.com-20111221011100-gybt9pjdfpg4wc86 --- openerp/addons/base/test/base_test.yml | 20 ++++++++++++++++++++ openerp/osv/fields.py | 17 +++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/openerp/addons/base/test/base_test.yml b/openerp/addons/base/test/base_test.yml index fcead834d93..450719af046 100644 --- a/openerp/addons/base/test/base_test.yml +++ b/openerp/addons/base/test/base_test.yml @@ -267,6 +267,26 @@ try_round(457.5, '460.00', precision_rounding=5) try_round(457.1, '456.00', precision_rounding=3) +- + "Float precision tests: check that proper rounding is performed for float persistence" +- + !python {model: res.currency}: | + currency = self.browse(cr, uid, ref('base.EUR')) + res_currency_rate = self.pool.get('res.currency.rate') + from tools import float_compare, float_is_zero, float_round, float_repr + def try_roundtrip(value, expected, self=self, cr=cr, currency=currency, + res_currency_rate=res_currency_rate): + rate_id = res_currency_rate.create(cr, 1, {'name':'2000-01-01', + 'rate': value, + 'currency_id': currency.id}) + rate = res_currency_rate.read(cr, 1, rate_id, ['rate'])['rate'] + assert rate == expected, 'Roundtrip error: got %s back from db, expected %s' % (rate, expected) + # res.currency.rate uses 6 digits of precision by default + try_roundtrip(2.6748955, 2.674896) + try_roundtrip(-2.6748955, -2.674896) + try_roundtrip(10000.999999, 10000.999999) + try_roundtrip(-10000.999999, -10000.999999) + - "Float precision tests: verify that invalid parameters are forbidden" - diff --git a/openerp/osv/fields.py b/openerp/osv/fields.py index ea1b6d93329..5a40eaff5fa 100644 --- a/openerp/osv/fields.py +++ b/openerp/osv/fields.py @@ -239,8 +239,10 @@ class float(_column): def digits_change(self, cr): if self.digits_compute: self.digits = self.digits_compute(cr) + if self.digits: precision, scale = self.digits - self._symbol_set = ('%s', lambda x: float_repr(float_round(__builtin__.float(x or 0.0), precision_digits=scale), + self._symbol_set = ('%s', lambda x: float_repr(float_round(__builtin__.float(x or 0.0), + precision_digits=scale), precision_digits=scale)) class date(_column): @@ -992,11 +994,14 @@ class function(_column): self._symbol_set = integer._symbol_set def digits_change(self, cr): - if self.digits_compute: - self.digits = self.digits_compute(cr) - precision, scale = self.digits - self._symbol_set = ('%s', lambda x: float_repr(float_round(__builtin__.float(x or 0.0), precision_digits=scale), - precision_digits=scale)) + if self._type == 'float': + if self.digits_compute: + self.digits = self.digits_compute(cr) + if self.digits: + precision, scale = self.digits + self._symbol_set = ('%s', lambda x: float_repr(float_round(__builtin__.float(x or 0.0), + precision_digits=scale), + precision_digits=scale)) def search(self, cr, uid, obj, name, args, context=None): if not self._fnct_search: