From d4972ffdb6b9356a524eef1dbc11f455ff4473f2 Mon Sep 17 00:00:00 2001 From: Cedric Snauwaert Date: Tue, 23 Sep 2014 17:39:14 +0200 Subject: [PATCH 1/4] [FIX] product,float_utils: perform ceiling via float_round with new rounding_method UP Modified product ceiling() to use float_round() with special mode for rounding UP (away from zero), avoiding pathological cases where float representations errors were ceiling to the superior unit. Also added correspding tests for rounding_method=UP Fixes issue #1125, and replaces PR #1126. --- addons/product/_common.py | 5 +---- openerp/addons/base/test/base_test.yml | 13 ++++++++++-- openerp/tools/float_utils.py | 29 +++++++++++++++++++------- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/addons/product/_common.py b/addons/product/_common.py index c05dcee66a2..f44f6b11a4c 100644 --- a/addons/product/_common.py +++ b/addons/product/_common.py @@ -20,9 +20,6 @@ ############################################################################## from openerp import tools -import math - - def rounding(f, r): # TODO for trunk: log deprecation warning # _logger.warning("Deprecated rounding method, please use tools.float_round to round floats.") @@ -32,4 +29,4 @@ def rounding(f, r): def ceiling(f, r): if not r: return f - return math.ceil(f / r) * r + return tools.float_round(f, precision_rounding=r, rounding_method='UP') diff --git a/openerp/addons/base/test/base_test.yml b/openerp/addons/base/test/base_test.yml index 11f243592fa..5a30bc22529 100644 --- a/openerp/addons/base/test/base_test.yml +++ b/openerp/addons/base/test/base_test.yml @@ -198,8 +198,8 @@ - !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), + def try_round(amount, expected, precision_digits=3, float_round=float_round, float_repr=float_repr, rounding_method='HALF-UP'): + result = float_repr(float_round(amount, precision_digits=precision_digits, rounding_method=rounding_method), precision_digits=precision_digits) assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected) try_round(2.6745, '2.675') @@ -213,6 +213,15 @@ try_round(457.4554, '457.455') try_round(-457.4554, '-457.455') + # Try some rounding value with rounding method UP instead of HALF-UP + # We use 8.175 because when normalizing 8.175 with precision_digits=3 it gives + # us 8175,0000000001234 as value, and if not handle correctly the rounding UP + # value will be incorrect (should be 8,175 and not 8,176) + try_round(8.175, '8.175', rounding_method='UP') + try_round(8.1751, '8.176', rounding_method='UP') + try_round(-8.175, '-8.175', rounding_method='UP') + try_round(-8.1751, '-8.175', rounding_method='UP') + # 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'] diff --git a/openerp/tools/float_utils.py b/openerp/tools/float_utils.py index 5c934114e3c..79f5a34c7f6 100644 --- a/openerp/tools/float_utils.py +++ b/openerp/tools/float_utils.py @@ -29,10 +29,11 @@ def _float_check_precision(precision_digits=None, precision_rounding=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. +def float_round(value, precision_digits=None, precision_rounding=None, rounding_method='HALF-UP'): + """Return ``value`` rounded to ``precision_digits`` decimal digits, + minimizing IEEE-754 floating point representation errors, and applying + the tie-breaking rule selected with ``rounding_method``, by default + HALF-UP (away from zero). Precision must be given by ``precision_digits`` or ``precision_rounding``, not both! @@ -41,6 +42,9 @@ def float_round(value, 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 rounding_method: the rounding method used: 'HALF-UP' or 'UP', the first + one rounding up to the closest number with the rule that number>=0.5 is + rounded up to 1, and the latest one always rounding up. :return: rounded float """ rounding_factor = _float_check_precision(precision_digits=precision_digits, @@ -52,7 +56,7 @@ def float_round(value, precision_digits=None, precision_rounding=None): # 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 + # TIE-BREAKING: HALF-UP (for normal rounding) # 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 @@ -66,8 +70,19 @@ def float_round(value, precision_digits=None, precision_rounding=None): 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 + if rounding_method == 'HALF-UP': + normalized_value += cmp(normalized_value,0) * epsilon + rounded_value = round(normalized_value) # round to integer + + # TIE-BREAKING: UP (for ceiling operations) + # When rounding the value up, we instead subtract the epsilon value + # as the the approximation of the real value may be slightly *above* the + # tie limit, this would result in incorrectly rounding up to the next number + + elif rounding_method == 'UP': + normalized_value -= cmp(normalized_value,0) * epsilon + rounded_value = math.ceil(normalized_value) # ceil to integer + result = rounded_value * rounding_factor # de-normalize return result From 311c77bb885d815aeb64ac5e89d3665f4c2dd172 Mon Sep 17 00:00:00 2001 From: Cedric Snauwaert Date: Wed, 24 Sep 2014 16:09:28 +0200 Subject: [PATCH 2/4] [FIX] product: _compute_qty: first round before ceiling, to avoid pathological cases Fixes problem when we try to sell 12 units of a product and change it to 1 dozen, the algorithm was then trying to recompute the original amount and was getting 12,0000048 as a result which was then passed to the ceiling method, getting 13.0! See also previous commit and issue #1125, PR #1126 --- addons/product/product.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/addons/product/product.py b/addons/product/product.py index 83ab8183f7a..da0ce6f9040 100644 --- a/addons/product/product.py +++ b/addons/product/product.py @@ -29,6 +29,7 @@ from openerp.osv import osv, fields from openerp.tools.translate import _ import openerp.addons.decimal_precision as dp +from openerp.tools.float_utils import float_round def ean_checksum(eancode): """returns the checksum of an ean string of length 13, returns -1 if the string has the wrong length""" @@ -176,7 +177,10 @@ class product_uom(osv.osv): raise osv.except_osv(_('Error!'), _('Conversion from Product UoM %s to Default UoM %s is not possible as they both belong to different Category!.') % (from_unit.name,to_unit.name,)) else: return qty - amount = qty / from_unit.factor + # First round to the precision of the original unit, so that + # float representation errors do not bias the following ceil() + # e.g. with 1 / (1/12) we could get 12.0000048, ceiling to 13! + amount = float_round(qty/from_unit.factor, precision_rounding=from_unit.rounding) if to_unit: amount = ceiling(amount * to_unit.factor, to_unit.rounding) return amount From 7b17133b532c5023135d8e4254734a92df4ac6d6 Mon Sep 17 00:00:00 2001 From: Christophe Simonis Date: Thu, 25 Sep 2014 11:51:15 +0200 Subject: [PATCH 3/4] [FIX] portal_sale: force empty context The context was removed by 1933e926. --- addons/portal_sale/portal_sale_view.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/addons/portal_sale/portal_sale_view.xml b/addons/portal_sale/portal_sale_view.xml index 2bdc41c09f9..80dff872377 100644 --- a/addons/portal_sale/portal_sale_view.xml +++ b/addons/portal_sale/portal_sale_view.xml @@ -64,6 +64,7 @@ account.invoice tree,form [('type','in',['out_invoice','out_refund'])] + {} We haven't sent you any invoice. From 5f6d324db64f54402c545823fb9039f3c2ec7805 Mon Sep 17 00:00:00 2001 From: Simon Lejeune Date: Thu, 25 Sep 2014 13:40:45 +0200 Subject: [PATCH 4/4] [FIX] crm: merge the phonecalls during an opportunities merge --- addons/crm/crm_lead.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/addons/crm/crm_lead.py b/addons/crm/crm_lead.py index b04eae9e31f..00c6f931ee3 100644 --- a/addons/crm/crm_lead.py +++ b/addons/crm/crm_lead.py @@ -626,6 +626,13 @@ class crm_lead(base_stage, format_address, osv.osv): attachment.write(values) return True + def _merge_opportunity_phonecalls(self, cr, uid, opportunity_id, opportunities, context=None): + phonecall_obj = self.pool['crm.phonecall'] + for opportunity in opportunities: + for phonecall_id in phonecall_obj.search(cr, uid, [('opportunity_id', '=', opportunity.id)], context=context): + phonecall_obj.write(cr, uid, phonecall_id, {'opportunity_id': opportunity_id}, context=context) + return True + def merge_opportunity(self, cr, uid, ids, context=None): """ Different cases of merge: @@ -663,6 +670,7 @@ class crm_lead(base_stage, format_address, osv.osv): # Merge messages and attachements into the first opportunity self._merge_opportunity_history(cr, uid, highest.id, tail_opportunities, context=context) self._merge_opportunity_attachments(cr, uid, highest.id, tail_opportunities, context=context) + self._merge_opportunity_phonecalls(cr, uid, highest.id, tail_opportunities, context=context) # Merge notifications about loss of information opportunities = [highest]