diff --git a/openerp/addons/base/res/res_currency.py b/openerp/addons/base/res/res_currency.py index 32aa387e828..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 +from tools import float_round, float_is_zero, float_compare from tools.translate import _ CURRENCY_DISPLAY_PATTERN = re.compile(r'(\w+)\s*(?:\((.*)\))?') @@ -127,15 +127,49 @@ 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. + + :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`` 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 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 + :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. + + 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 + """ + 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/addons/base/test/base_test.yml b/openerp/addons/base/test/base_test.yml index 6ea578a8659..450719af046 100644 --- a/openerp/addons/base/test/base_test.yml +++ b/openerp/addons/base/test/base_test.yml @@ -144,3 +144,163 @@ !python {model: res.partner.category}: | self.pool._init = True +- + "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, 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.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 + + 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(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(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(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, 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.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') + 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 = ['.00', '.02', '.01', '.68', '.67', '.46', '.456', '.4556'] + precisions = [2, 2, 2, 2, 2, 2, 3, 4] + # 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,97): + 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 + 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(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(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) + + # 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.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: 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" +- + !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: + 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/osv/fields.py b/openerp/osv/fields.py index 42bc9b75c1d..5a40eaff5fa 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, float_repr def _symbol_set(symb): if symb == None or symb == False: @@ -229,17 +230,20 @@ 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) + 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)) class date(_column): _type = 'date' @@ -990,11 +994,14 @@ class function(_column): self._symbol_set = integer._symbol_set 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 - + 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: 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..5c934114e3c --- /dev/null +++ b/openerp/tools/float_utils.py @@ -0,0 +1,186 @@ +# -*- 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 . +# +############################################################################## + +import math + +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, and applying HALF-UP (away from zero) tie-breaking rule. + Precision must be given by ``precision_digits`` or ``precision_rounding``, + not both! + + :param float value: the value to round + :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). + :return: rounded float + """ + rounding_factor = _float_check_precision(precision_digits=precision_digits, + precision_rounding=precision_rounding) + if rounding_factor == 0 or value == 0: return 0.0 + + # 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 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! + + 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 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 the precision's zero + :return: True if ``value`` is considered zero + """ + epsilon = _float_check_precision(precision_digits=precision_digits, + precision_rounding=precision_rounding) + 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! + 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 (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 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 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 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 + +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__": + + import time + start = time.time() + count = 0 + errors = 0 + + def try_round(amount, expected, precision_digits=3): + global count, errors; count += 1 + result = float_repr(float_round(amount, precision_digits=precision_digits), + precision_digits=precision_digits) + if result != expected: + errors += 1 + 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 = ['.00', '.02', '.01', '.68', '.67', '.46', '.456', '.4556'] + precisions = [2, 2, 2, 2, 2, 2, 3, 4] + 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,97): + 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' diff --git a/openerp/tools/misc.py b/openerp/tools/misc.py index dbccbe5b217..7fdb2f07386 100644 --- a/openerp/tools/misc.py +++ b/openerp/tools/misc.py @@ -1200,4 +1200,4 @@ class UnquoteEvalContext(defaultdict): def __missing__(self, key): return unquote(key) -# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: +# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: \ No newline at end of file