Merge lp:~openerp-dev/openobject-server/trunk-float-rounding-odo into lp:openobject-server
- trunk-float-rounding-odo
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 3911 |
Proposed branch: | lp:~openerp-dev/openobject-server/trunk-float-rounding-odo |
Merge into: | lp:openobject-server |
Diff against target: |
508 lines (+406/-18) 6 files modified
openerp/addons/base/res/res_currency.py (+42/-8) openerp/addons/base/test/base_test.yml (+160/-0) openerp/osv/fields.py (+16/-9) openerp/tools/__init__.py (+1/-0) openerp/tools/float_utils.py (+186/-0) openerp/tools/misc.py (+1/-1) |
To merge this branch: | bzr merge lp:~openerp-dev/openobject-server/trunk-float-rounding-odo |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Vo Minh Thu (community) | Approve | ||
Lorenzo Battistini (community) | Abstain | ||
Review via email:
|
Commit message
Description of the change
- Extracted res.currency's float computation methods to tools, so they can be reused cleanly for other floating-point operations.
- Also added a compare() method to allow for easy comparison of 2 float values, as suggested by Ferdinand on bug 865387
- Added tests and docstrings to explain the logic.
Watch out for compare(), because compare(
The utility methods used in the YAML tests take redundant arguments due to idiotic namespace issues, feel free to suggest alternatives.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ferdinand (office-chricar) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Xavier (Open ERP) (xmo-deactivatedaccount) wrote : | # |
That's kind-of nitpicky, but in `float_is_zero` I think `rounding_factor` should be renamed `epsilon`, as that's the term usually used for floating-point relative errors: http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Cloves Almeida (cjalmeida) wrote : | # |
"misc.py" is over 1500 lines already. Since this is to be used everywhere, maybe a "tools/float.py" is a better approach?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ferdinand (office-chricar) wrote : | # |
Have checked with data from
https:/
and it works nicely
I attach a patched for base_test.yml with extended tests and a bit more verboseitiy
here
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lorenzo Battistini (elbati) wrote : | # |
Hello Olivier,
I think I found an issue:
float_round(
17.35
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Olivier Dony (Odoo) (odo-openerp) wrote : | # |
> I think I found an issue:
>
> float_round(
> 17.35
That's the expected result as far as I can see. Perhaps you meant to test with precision_
>>> from decimal import Decimal, ROUND_HALF_UP
>>> Decimal(
Decimal('17.35')
>>> Decimal(
Decimal('17.4')
>>>
Anyway the code still needs to be updated to cover more tricky half-up cases, as discussed on bug 882036.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Olivier Dony (Odoo) (odo-openerp) wrote : | # |
> "misc.py" is over 1500 lines already. Since this is to be used everywhere,
> maybe a "tools/float.py" is a better approach?
Good point, moved it to tools/float_
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lorenzo Battistini (elbati) wrote : | # |
>>
>> float_round(
>> 17.35
>
> That's the expected result as far as I can see.
Sorry Olivier, you are right. I wrote it too quickly and had another
thing in mind.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lorenzo Battistini (elbati) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Olivier Dony (Odoo) (odo-openerp) wrote : | # |
I think I'm done with the final changes here:
- tie-breaking epsilon has been implemented similarly to what was discussed on bug bug 882036 in order to perform proper HALF-UP rounding
- an additional float_repr method was added in float_utils to render a float to a str, as the default str() and repr() do not do what we need. repr() will not round appropriately, and str() will round too eagerly when there are more than 12 significant digits, for some reason.
- the ORM has been updated to properly apply rounding before persisting a value (this is really what bug 882036 was about). It passes the float values to postgres using float_repr, to avoid any precision loss.
- YAML tests updated to include thousands of cases from low to high magnitudes + a test of float round-trip via the database too
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Vo Minh Thu (thu) wrote : | # |
I haven't spotted anything obviously stupid.
Tests could be moved to openerp.tests but this can wait until the runbot really run those.
Preview Diff
1 | === modified file 'openerp/addons/base/res/res_currency.py' |
2 | --- openerp/addons/base/res/res_currency.py 2011-10-11 16:34:35 +0000 |
3 | +++ openerp/addons/base/res/res_currency.py 2011-12-21 01:14:24 +0000 |
4 | @@ -24,7 +24,7 @@ |
5 | from osv import fields, osv |
6 | import tools |
7 | |
8 | -from tools.misc import currency |
9 | +from tools import float_round, float_is_zero, float_compare |
10 | from tools.translate import _ |
11 | |
12 | CURRENCY_DISPLAY_PATTERN = re.compile(r'(\w+)\s*(?:\((.*)\))?') |
13 | @@ -127,15 +127,49 @@ |
14 | return [(x['id'], tools.ustr(x['name']) + (x['symbol'] and (' (' + tools.ustr(x['symbol']) + ')') or '')) for x in reads] |
15 | |
16 | def round(self, cr, uid, currency, amount): |
17 | - if currency.rounding == 0: |
18 | - return 0.0 |
19 | - else: |
20 | - # /!\ First member below must be rounded to full unit! |
21 | - # Do not pass a rounding digits value to round() |
22 | - return round(amount / currency.rounding) * currency.rounding |
23 | + """Return ``amount`` rounded according to ``currency``'s |
24 | + rounding rules. |
25 | + |
26 | + :param browse_record currency: currency for which we are rounding |
27 | + :param float amount: the amount to round |
28 | + :return: rounded float |
29 | + """ |
30 | + return float_round(amount, precision_rounding=currency.rounding) |
31 | + |
32 | + def compare_amounts(self, cr, uid, currency, amount1, amount2): |
33 | + """Compare ``amount1`` and ``amount2`` after rounding them according to the |
34 | + given currency's precision.. |
35 | + An amount is considered lower/greater than another amount if their rounded |
36 | + value is different. This is not the same as having a non-zero difference! |
37 | + |
38 | + For example 1.432 and 1.431 are equal at 2 digits precision, |
39 | + so this method would return 0. |
40 | + However 0.006 and 0.002 are considered different (returns 1) because |
41 | + they respectively round to 0.01 and 0.0, even though |
42 | + 0.006-0.002 = 0.004 which would be considered zero at 2 digits precision. |
43 | + |
44 | + :param browse_record currency: currency for which we are rounding |
45 | + :param float amount1: first amount to compare |
46 | + :param float amount2: second amount to compare |
47 | + :return: (resp.) -1, 0 or 1, if ``amount1`` is (resp.) lower than, |
48 | + equal to, or greater than ``amount2``, according to |
49 | + ``currency``'s rounding. |
50 | + """ |
51 | + return float_compare(amount1, amount2, precision_rounding=currency.rounding) |
52 | |
53 | def is_zero(self, cr, uid, currency, amount): |
54 | - return abs(self.round(cr, uid, currency, amount)) < currency.rounding |
55 | + """Returns true if ``amount`` is small enough to be treated as |
56 | + zero according to ``currency``'s rounding rules. |
57 | + |
58 | + Warning: ``is_zero(amount1-amount2)`` is not always equivalent to |
59 | + ``compare_amounts(amount1,amount2) == 0``, as the former will round after |
60 | + computing the difference, while the latter will round before, giving |
61 | + different results for e.g. 0.006 and 0.002 at 2 digits precision. |
62 | + |
63 | + :param browse_record currency: currency for which we are rounding |
64 | + :param float amount: amount to compare with currency's zero |
65 | + """ |
66 | + return float_is_zero(amount, precision_rounding=currency.rounding) |
67 | |
68 | def _get_conversion_rate(self, cr, uid, from_currency, to_currency, context=None): |
69 | if context is None: |
70 | |
71 | === modified file 'openerp/addons/base/test/base_test.yml' |
72 | --- openerp/addons/base/test/base_test.yml 2011-07-28 08:27:52 +0000 |
73 | +++ openerp/addons/base/test/base_test.yml 2011-12-21 01:14:24 +0000 |
74 | @@ -144,3 +144,163 @@ |
75 | !python {model: res.partner.category}: | |
76 | self.pool._init = True |
77 | |
78 | +- |
79 | + "Float precision tests: verify that float rounding methods are working correctly via res.currency" |
80 | +- |
81 | + !python {model: res.currency}: | |
82 | + from tools import float_repr |
83 | + from math import log10 |
84 | + currency = self.browse(cr, uid, ref('base.EUR')) |
85 | + def try_round(amount, expected, self=self, cr=cr, currency=currency, float_repr=float_repr, |
86 | + log10=log10): |
87 | + digits = max(0,-int(log10(currency.rounding))) |
88 | + result = float_repr(self.round(cr, 1, currency, amount), precision_digits=digits) |
89 | + assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) |
90 | + try_round(2.674,'2.67') |
91 | + try_round(2.675,'2.68') # in Python 2.7.2, round(2.675,2) gives 2.67 |
92 | + try_round(-2.675,'-2.68') # in Python 2.7.2, round(2.675,2) gives 2.67 |
93 | + try_round(0.001,'0.00') |
94 | + try_round(-0.001,'-0.00') |
95 | + try_round(0.0049,'0.00') # 0.0049 is closer to 0 than to 0.01, so should round down |
96 | + try_round(0.005,'0.01') # the rule is to round half away from zero |
97 | + try_round(-0.005,'-0.01') # the rule is to round half away from zero |
98 | + |
99 | + def try_zero(amount, expected, self=self, cr=cr, currency=currency): |
100 | + assert self.is_zero(cr, 1, currency, amount) == expected, "Rounding error: %s should be zero!" % amount |
101 | + try_zero(0.01, False) |
102 | + try_zero(-0.01, False) |
103 | + try_zero(0.001, True) |
104 | + try_zero(-0.001, True) |
105 | + try_zero(0.0046, True) |
106 | + try_zero(-0.0046, True) |
107 | + try_zero(2.68-2.675, False) # 2.68 - 2.675 = 0.005 -> rounds to 0.01 |
108 | + try_zero(2.68-2.676, True) # 2.68 - 2.675 = 0.004 -> rounds to 0.0 |
109 | + try_zero(2.676-2.68, True) # 2.675 - 2.68 = -0.004 -> rounds to -0.0 |
110 | + try_zero(2.675-2.68, False) # 2.675 - 2.68 = -0.005 -> rounds to -0.01 |
111 | + |
112 | + def try_compare(amount1, amount2, expected, self=self, cr=cr, currency=currency): |
113 | + assert self.compare_amounts(cr, 1, currency, amount1, amount2) == expected, \ |
114 | + "Rounding error, compare_amounts(%s,%s) should be %s" % (amount1, amount2, expected) |
115 | + try_compare(0.001, 0.001, 0) |
116 | + try_compare(-0.001, -0.001, 0) |
117 | + try_compare(0.001, 0.002, 0) |
118 | + try_compare(-0.001, -0.002, 0) |
119 | + try_compare(2.675, 2.68, 0) |
120 | + try_compare(2.676, 2.68, 0) |
121 | + try_compare(-2.676, -2.68, 0) |
122 | + try_compare(2.674, 2.68, -1) |
123 | + try_compare(-2.674, -2.68, 1) |
124 | + try_compare(3, 2.68, 1) |
125 | + try_compare(-3, -2.68, -1) |
126 | + try_compare(0.01, 0, 1) |
127 | + try_compare(-0.01, 0, -1) |
128 | + |
129 | +- |
130 | + "Float precision tests: verify that float rounding methods are working correctly via tools" |
131 | +- |
132 | + !python {model: res.currency}: | |
133 | + from tools import float_compare, float_is_zero, float_round, float_repr |
134 | + def try_round(amount, expected, precision_digits=3, float_round=float_round, float_repr=float_repr): |
135 | + result = float_repr(float_round(amount, precision_digits=precision_digits), |
136 | + precision_digits=precision_digits) |
137 | + assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) |
138 | + try_round(2.6745, '2.675') |
139 | + try_round(-2.6745, '-2.675') |
140 | + try_round(2.6744, '2.674') |
141 | + try_round(-2.6744, '-2.674') |
142 | + try_round(0.0004, '0.000') |
143 | + try_round(-0.0004, '-0.000') |
144 | + try_round(357.4555, '357.456') |
145 | + try_round(-357.4555, '-357.456') |
146 | + try_round(457.4554, '457.455') |
147 | + try_round(-457.4554, '-457.455') |
148 | + |
149 | + # Extended float range test, inspired by Cloves Almeida's test on bug #882036. |
150 | + fractions = [.0, .015, .01499, .675, .67499, .4555, .4555, .45555] |
151 | + expecteds = ['.00', '.02', '.01', '.68', '.67', '.46', '.456', '.4556'] |
152 | + precisions = [2, 2, 2, 2, 2, 2, 3, 4] |
153 | + # Note: max precision for double floats is 53 bits of precision or |
154 | + # 17 significant decimal digits |
155 | + for magnitude in range(7): |
156 | + for i in xrange(len(fractions)): |
157 | + frac, exp, prec = fractions[i], expecteds[i], precisions[i] |
158 | + for sign in [-1,1]: |
159 | + for x in xrange(0,10000,97): |
160 | + n = x * 10**magnitude |
161 | + f = sign * (n + frac) |
162 | + f_exp = ('-' if f != 0 and sign == -1 else '') + str(n) + exp |
163 | + try_round(f, f_exp, precision_digits=prec) |
164 | + |
165 | + |
166 | + def try_zero(amount, expected, float_is_zero=float_is_zero): |
167 | + assert float_is_zero(amount, precision_digits=3) == expected, "Rounding error: %s should be zero!" % amount |
168 | + try_zero(0.0002, True) |
169 | + try_zero(-0.0002, True) |
170 | + try_zero(0.00034, True) |
171 | + try_zero(0.0005, False) |
172 | + try_zero(-0.0005, False) |
173 | + try_zero(0.0008, False) |
174 | + try_zero(-0.0008, False) |
175 | + |
176 | + def try_compare(amount1, amount2, expected, float_compare=float_compare): |
177 | + assert float_compare(amount1, amount2, precision_digits=3) == expected, \ |
178 | + "Rounding error, compare_amounts(%s,%s) should be %s" % (amount1, amount2, expected) |
179 | + try_compare(0.0003, 0.0004, 0) |
180 | + try_compare(-0.0003, -0.0004, 0) |
181 | + try_compare(0.0002, 0.0005, -1) |
182 | + try_compare(-0.0002, -0.0005, 1) |
183 | + try_compare(0.0009, 0.0004, 1) |
184 | + try_compare(-0.0009, -0.0004, -1) |
185 | + try_compare(557.4555, 557.4556, 0) |
186 | + try_compare(-557.4555, -557.4556, 0) |
187 | + try_compare(657.4444, 657.445, -1) |
188 | + try_compare(-657.4444, -657.445, 1) |
189 | + |
190 | + # Rounding to unusual rounding units (e.g. coin values) |
191 | + def try_round(amount, expected, precision_rounding=None, float_round=float_round, float_repr=float_repr): |
192 | + result = float_repr(float_round(amount, precision_rounding=precision_rounding), |
193 | + precision_digits=2) |
194 | + assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) |
195 | + try_round(-457.4554, '-457.45', precision_rounding=0.05) |
196 | + try_round(457.444, '457.50', precision_rounding=0.5) |
197 | + try_round(457.3, '455.00', precision_rounding=5) |
198 | + try_round(457.5, '460.00', precision_rounding=5) |
199 | + try_round(457.1, '456.00', precision_rounding=3) |
200 | + |
201 | +- |
202 | + "Float precision tests: check that proper rounding is performed for float persistence" |
203 | +- |
204 | + !python {model: res.currency}: | |
205 | + currency = self.browse(cr, uid, ref('base.EUR')) |
206 | + res_currency_rate = self.pool.get('res.currency.rate') |
207 | + from tools import float_compare, float_is_zero, float_round, float_repr |
208 | + def try_roundtrip(value, expected, self=self, cr=cr, currency=currency, |
209 | + res_currency_rate=res_currency_rate): |
210 | + rate_id = res_currency_rate.create(cr, 1, {'name':'2000-01-01', |
211 | + 'rate': value, |
212 | + 'currency_id': currency.id}) |
213 | + rate = res_currency_rate.read(cr, 1, rate_id, ['rate'])['rate'] |
214 | + assert rate == expected, 'Roundtrip error: got %s back from db, expected %s' % (rate, expected) |
215 | + # res.currency.rate uses 6 digits of precision by default |
216 | + try_roundtrip(2.6748955, 2.674896) |
217 | + try_roundtrip(-2.6748955, -2.674896) |
218 | + try_roundtrip(10000.999999, 10000.999999) |
219 | + try_roundtrip(-10000.999999, -10000.999999) |
220 | + |
221 | +- |
222 | + "Float precision tests: verify that invalid parameters are forbidden" |
223 | +- |
224 | + !python {model: res.currency}: | |
225 | + from tools import float_compare, float_is_zero, float_round |
226 | + try: |
227 | + float_is_zero(0.01, precision_digits=3, precision_rounding=0.01) |
228 | + except AssertionError: |
229 | + pass |
230 | + try: |
231 | + float_compare(0.01, 0.02, precision_digits=3, precision_rounding=0.01) |
232 | + except AssertionError: |
233 | + pass |
234 | + try: |
235 | + float_round(0.01, precision_digits=3, precision_rounding=0.01) |
236 | + except AssertionError: |
237 | + pass |
238 | |
239 | === modified file 'openerp/osv/fields.py' |
240 | --- openerp/osv/fields.py 2011-11-28 12:45:35 +0000 |
241 | +++ openerp/osv/fields.py 2011-12-21 01:14:24 +0000 |
242 | @@ -45,6 +45,7 @@ |
243 | import openerp.netsvc as netsvc |
244 | import openerp.tools as tools |
245 | from openerp.tools.translate import _ |
246 | +from openerp.tools import float_round, float_repr |
247 | |
248 | def _symbol_set(symb): |
249 | if symb == None or symb == False: |
250 | @@ -229,17 +230,20 @@ |
251 | def __init__(self, string='unknown', digits=None, digits_compute=None, required=False, **args): |
252 | _column.__init__(self, string=string, required=required, **args) |
253 | self.digits = digits |
254 | + # synopsis: digits_compute(cr) -> (precision, scale) |
255 | self.digits_compute = digits_compute |
256 | if required: |
257 | warnings.warn("Making a float field `required` has no effect, as NULL values are " |
258 | "automatically turned into 0.0", PendingDeprecationWarning, stacklevel=2) |
259 | |
260 | - |
261 | def digits_change(self, cr): |
262 | if self.digits_compute: |
263 | - t = self.digits_compute(cr) |
264 | - self._symbol_set=('%s', lambda x: ('%.'+str(t[1])+'f') % (__builtin__.float(x or 0.0),)) |
265 | - self.digits = t |
266 | + self.digits = self.digits_compute(cr) |
267 | + if self.digits: |
268 | + precision, scale = self.digits |
269 | + self._symbol_set = ('%s', lambda x: float_repr(float_round(__builtin__.float(x or 0.0), |
270 | + precision_digits=scale), |
271 | + precision_digits=scale)) |
272 | |
273 | class date(_column): |
274 | _type = 'date' |
275 | @@ -990,11 +994,14 @@ |
276 | self._symbol_set = integer._symbol_set |
277 | |
278 | def digits_change(self, cr): |
279 | - if self.digits_compute: |
280 | - t = self.digits_compute(cr) |
281 | - self._symbol_set=('%s', lambda x: ('%.'+str(t[1])+'f') % (__builtin__.float(x or 0.0),)) |
282 | - self.digits = t |
283 | - |
284 | + if self._type == 'float': |
285 | + if self.digits_compute: |
286 | + self.digits = self.digits_compute(cr) |
287 | + if self.digits: |
288 | + precision, scale = self.digits |
289 | + self._symbol_set = ('%s', lambda x: float_repr(float_round(__builtin__.float(x or 0.0), |
290 | + precision_digits=scale), |
291 | + precision_digits=scale)) |
292 | |
293 | def search(self, cr, uid, obj, name, args, context=None): |
294 | if not self._fnct_search: |
295 | |
296 | === modified file 'openerp/tools/__init__.py' |
297 | --- openerp/tools/__init__.py 2011-06-23 09:03:57 +0000 |
298 | +++ openerp/tools/__init__.py 2011-12-21 01:14:24 +0000 |
299 | @@ -31,6 +31,7 @@ |
300 | from pdf_utils import * |
301 | from yaml_import import * |
302 | from sql import * |
303 | +from float_utils import * |
304 | |
305 | #.apidoc title: Tools |
306 | |
307 | |
308 | === added file 'openerp/tools/float_utils.py' |
309 | --- openerp/tools/float_utils.py 1970-01-01 00:00:00 +0000 |
310 | +++ openerp/tools/float_utils.py 2011-12-21 01:14:24 +0000 |
311 | @@ -0,0 +1,186 @@ |
312 | +# -*- coding: utf-8 -*- |
313 | +############################################################################## |
314 | +# |
315 | +# OpenERP, Open Source Business Applications |
316 | +# Copyright (c) 2011 OpenERP S.A. <http://openerp.com> |
317 | +# |
318 | +# This program is free software: you can redistribute it and/or modify |
319 | +# it under the terms of the GNU Affero General Public License as |
320 | +# published by the Free Software Foundation, either version 3 of the |
321 | +# License, or (at your option) any later version. |
322 | +# |
323 | +# This program is distributed in the hope that it will be useful, |
324 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
325 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
326 | +# GNU Affero General Public License for more details. |
327 | +# |
328 | +# You should have received a copy of the GNU Affero General Public License |
329 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
330 | +# |
331 | +############################################################################## |
332 | + |
333 | +import math |
334 | + |
335 | +def _float_check_precision(precision_digits=None, precision_rounding=None): |
336 | + assert (precision_digits is not None or precision_rounding is not None) and \ |
337 | + not (precision_digits and precision_rounding),\ |
338 | + "exactly one of precision_digits and precision_rounding must be specified" |
339 | + if precision_digits is not None: |
340 | + return 10 ** -precision_digits |
341 | + return precision_rounding |
342 | + |
343 | +def float_round(value, precision_digits=None, precision_rounding=None): |
344 | + """Return ``value`` rounded to ``precision_digits`` |
345 | + decimal digits, minimizing IEEE-754 floating point representation |
346 | + errors, and applying HALF-UP (away from zero) tie-breaking rule. |
347 | + Precision must be given by ``precision_digits`` or ``precision_rounding``, |
348 | + not both! |
349 | + |
350 | + :param float value: the value to round |
351 | + :param int precision_digits: number of fractional digits to round to. |
352 | + :param float precision_rounding: decimal number representing the minimum |
353 | + non-zero value at the desired precision (for example, 0.01 for a |
354 | + 2-digit precision). |
355 | + :return: rounded float |
356 | + """ |
357 | + rounding_factor = _float_check_precision(precision_digits=precision_digits, |
358 | + precision_rounding=precision_rounding) |
359 | + if rounding_factor == 0 or value == 0: return 0.0 |
360 | + |
361 | + # NORMALIZE - ROUND - DENORMALIZE |
362 | + # In order to easily support rounding to arbitrary 'steps' (e.g. coin values), |
363 | + # we normalize the value before rounding it as an integer, and de-normalize |
364 | + # after rounding: e.g. float_round(1.3, precision_rounding=.5) == 1.5 |
365 | + |
366 | + # TIE-BREAKING: HALF-UP |
367 | + # We want to apply HALF-UP tie-breaking rules, i.e. 0.5 rounds away from 0. |
368 | + # Due to IEE754 float/double representation limits, the approximation of the |
369 | + # real value may be slightly below the tie limit, resulting in an error of |
370 | + # 1 unit in the last place (ulp) after rounding. |
371 | + # For example 2.675 == 2.6749999999999998. |
372 | + # To correct this, we add a very small epsilon value, scaled to the |
373 | + # the order of magnitude of the value, to tip the tie-break in the right |
374 | + # direction. |
375 | + # Credit: discussion with OpenERP community members on bug 882036 |
376 | + |
377 | + normalized_value = value / rounding_factor # normalize |
378 | + epsilon_magnitude = math.log(abs(normalized_value), 2) |
379 | + epsilon = 2**(epsilon_magnitude-53) |
380 | + normalized_value += cmp(normalized_value,0) * epsilon |
381 | + rounded_value = round(normalized_value) # round to integer |
382 | + result = rounded_value * rounding_factor # de-normalize |
383 | + return result |
384 | + |
385 | +def float_is_zero(value, precision_digits=None, precision_rounding=None): |
386 | + """Returns true if ``value`` is small enough to be treated as |
387 | + zero at the given precision (smaller than the corresponding *epsilon*). |
388 | + The precision (``10**-precision_digits`` or ``precision_rounding``) |
389 | + is used as the zero *epsilon*: values less than that are considered |
390 | + to be zero. |
391 | + Precision must be given by ``precision_digits`` or ``precision_rounding``, |
392 | + not both! |
393 | + |
394 | + Warning: ``float_is_zero(value1-value2)`` is not equivalent to |
395 | + ``float_compare(value1,value2) == 0``, as the former will round after |
396 | + computing the difference, while the latter will round before, giving |
397 | + different results for e.g. 0.006 and 0.002 at 2 digits precision. |
398 | + |
399 | + :param int precision_digits: number of fractional digits to round to. |
400 | + :param float precision_rounding: decimal number representing the minimum |
401 | + non-zero value at the desired precision (for example, 0.01 for a |
402 | + 2-digit precision). |
403 | + :param float value: value to compare with the precision's zero |
404 | + :return: True if ``value`` is considered zero |
405 | + """ |
406 | + epsilon = _float_check_precision(precision_digits=precision_digits, |
407 | + precision_rounding=precision_rounding) |
408 | + return abs(float_round(value, precision_rounding=epsilon)) < epsilon |
409 | + |
410 | +def float_compare(value1, value2, precision_digits=None, precision_rounding=None): |
411 | + """Compare ``value1`` and ``value2`` after rounding them according to the |
412 | + given precision. A value is considered lower/greater than another value |
413 | + if their rounded value is different. This is not the same as having a |
414 | + non-zero difference! |
415 | + Precision must be given by ``precision_digits`` or ``precision_rounding``, |
416 | + not both! |
417 | + |
418 | + Example: 1.432 and 1.431 are equal at 2 digits precision, |
419 | + so this method would return 0 |
420 | + However 0.006 and 0.002 are considered different (this method returns 1) |
421 | + because they respectively round to 0.01 and 0.0, even though |
422 | + 0.006-0.002 = 0.004 which would be considered zero at 2 digits precision. |
423 | + |
424 | + Warning: ``float_is_zero(value1-value2)`` is not equivalent to |
425 | + ``float_compare(value1,value2) == 0``, as the former will round after |
426 | + computing the difference, while the latter will round before, giving |
427 | + different results for e.g. 0.006 and 0.002 at 2 digits precision. |
428 | + |
429 | + :param int precision_digits: number of fractional digits to round to. |
430 | + :param float precision_rounding: decimal number representing the minimum |
431 | + non-zero value at the desired precision (for example, 0.01 for a |
432 | + 2-digit precision). |
433 | + :param float value1: first value to compare |
434 | + :param float value2: second value to compare |
435 | + :return: (resp.) -1, 0 or 1, if ``value1`` is (resp.) lower than, |
436 | + equal to, or greater than ``value2``, at the given precision. |
437 | + """ |
438 | + rounding_factor = _float_check_precision(precision_digits=precision_digits, |
439 | + precision_rounding=precision_rounding) |
440 | + value1 = float_round(value1, precision_rounding=rounding_factor) |
441 | + value2 = float_round(value2, precision_rounding=rounding_factor) |
442 | + delta = value1 - value2 |
443 | + if float_is_zero(delta, precision_rounding=rounding_factor): return 0 |
444 | + return -1 if delta < 0.0 else 1 |
445 | + |
446 | +def float_repr(value, precision_digits): |
447 | + """Returns a string representation of a float with the |
448 | + the given number of fractional digits. This should not be |
449 | + used to perform a rounding operation (this is done via |
450 | + :meth:`~.float_round`), but only to produce a suitable |
451 | + string representation for a float. |
452 | + |
453 | + :param int precision_digits: number of fractional digits to |
454 | + include in the output |
455 | + """ |
456 | + # Can't use str() here because it seems to have an intrisic |
457 | + # rounding to 12 significant digits, which causes a loss of |
458 | + # precision. e.g. str(123456789.1234) == str(123456789.123)!! |
459 | + return ("%%.%sf" % precision_digits) % value |
460 | + |
461 | + |
462 | +if __name__ == "__main__": |
463 | + |
464 | + import time |
465 | + start = time.time() |
466 | + count = 0 |
467 | + errors = 0 |
468 | + |
469 | + def try_round(amount, expected, precision_digits=3): |
470 | + global count, errors; count += 1 |
471 | + result = float_repr(float_round(amount, precision_digits=precision_digits), |
472 | + precision_digits=precision_digits) |
473 | + if result != expected: |
474 | + errors += 1 |
475 | + print '###!!! Rounding error: got %s , expected %s' % (result, expected) |
476 | + |
477 | + # Extended float range test, inspired by Cloves Almeida's test on bug #882036. |
478 | + fractions = [.0, .015, .01499, .675, .67499, .4555, .4555, .45555] |
479 | + expecteds = ['.00', '.02', '.01', '.68', '.67', '.46', '.456', '.4556'] |
480 | + precisions = [2, 2, 2, 2, 2, 2, 3, 4] |
481 | + for magnitude in range(7): |
482 | + for i in xrange(len(fractions)): |
483 | + frac, exp, prec = fractions[i], expecteds[i], precisions[i] |
484 | + for sign in [-1,1]: |
485 | + for x in xrange(0,10000,97): |
486 | + n = x * 10**magnitude |
487 | + f = sign * (n + frac) |
488 | + f_exp = ('-' if f != 0 and sign == -1 else '') + str(n) + exp |
489 | + try_round(f, f_exp, precision_digits=prec) |
490 | + |
491 | + stop = time.time() |
492 | + |
493 | + # Micro-bench results: |
494 | + # 47130 round calls in 0.422306060791 secs, with Python 2.6.7 on Core i3 x64 |
495 | + # with decimal: |
496 | + # 47130 round calls in 6.612248100021 secs, with Python 2.6.7 on Core i3 x64 |
497 | + print count, " round calls, ", errors, "errors, done in ", (stop-start), 'secs' |
498 | |
499 | === modified file 'openerp/tools/misc.py' |
500 | --- openerp/tools/misc.py 2011-09-22 09:54:43 +0000 |
501 | +++ openerp/tools/misc.py 2011-12-21 01:14:24 +0000 |
502 | @@ -1200,4 +1200,4 @@ |
503 | def __missing__(self, key): |
504 | return unquote(key) |
505 | |
506 | -# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: |
507 | +# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: |
508 | \ No newline at end of file |
I hope this rounding methods will be used to replace the problematic checks
not an easy task to find all occurencies