Floating Point / Rounding issue in stock partial picking wizard

Bug #881356 reported by Marco Dieckhoff on 2011-10-25
22
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Fix Released
Medium
OpenERP R&D Addons Team 2
6.0
Confirmed
Medium
OpenERP Publisher's Warranty Team

Bug Description

We encountered an Floating Point rounding issue, in the Partial Picking wizard (stock/wizard/stock_partial_picking.py, do_partial)

"Processing quantity 390.268 kg for $PRODUCT is larger than the available quantity 390.268 kg."

(Decimal Precision of Product UoM is set to 3, UoM Rounding for kg is 0.001)

OK, so I added an output of the test

if calc_qty > move.move_id.product_qty:
390.268 > 390.268

As one would say, 390.268 > 390.268 should be false...

But:
calc_qty - move.move_id.product_qty => 390.268 - 390.268 is 5.68434188608e-14
This is beyond the default python %s rounding of 6 decimals (to understand the 390.268 > 290.268 output above)

So here is a real problem with rounding precision and float (instead of decimal) data type.

I changed the query to regard decimal precision:

prec = dp.get_precision('Product UoM')(cr)[1] or 0
if int(calc_qty * (10**prec)) > int(move.move_id.product_qty * (10**prec)):

and it works fine.

Related branches

Changed in openobject-addons:
assignee: nobody → OpenERP Publisher's Warranty Team (openerp-opw)
tags: added: maintenance
Ferdinand (office-chricar) wrote :

hello
wouldn't round(a,precision) = round(b,precision) do it ?

and

this is a potential trap for every float comparison ( ==, != ) which is based on python calculated values

Marco Dieckhoff (dieck) wrote :

Hm, round returns a float value again, so I first thought it would be the same problem over and over.

(I looked into these problems for some hours and was glad to finally have a workaround solution so we could continue work)

But rounding to a set precision should always return the same result for the same "decimal" representation, independent of the so comparism would probably work, yes.

See bug 865387 for similar problem and possible solution.
Maybe we should always use 'Decimal' for roundings?

Indeed, floating point values should not be compared with normal operators (except when they come directly from postgres), the usual way to do it is to subtract the 2 values and check the result against the precision limit.

For example, the res.currency object has a is_zero() method that will check that a given float value is small enough to be considered zero at the given currency precision, and can be used for all such comparisons.

The same technique should be applied here, subtract the 2 values and check that they are bigger than the "epsilon" for the current precision.

PS: for those wondering about the Decimal class, keep in mind that we don't use Decimal for OpenERP, our floating point calculations are all performed with floats, and we have appropriate solutions for all cases based on floats. So for consistency, Decimal use should not be introduced by mistake. For more info on the everlasting Float vs. Decimal discussion, see [1].

[1] https://lists.launchpad.net/openerp-expert-accounting/msg00067.html

Changed in openobject-addons:
assignee: OpenERP Publisher's Warranty Team (openerp-opw) → OpenERP R&D Addons Team 2 (openerp-dev-addons2)
importance: Undecided → Medium
status: New → Confirmed
milestone: none → 6.1
Changed in openobject-addons:
status: Confirmed → In Progress

Hello Marco Dieckhoff,

We have fixed the problem in lp:~openerp-dev/openobject-addons/trunk-bug-881356-han branch,
It will be merged soon in lp:openobject-addons.

Revision No: 5996
Revision ID: <email address hidden>

Thanks
HAN

Changed in openobject-addons:
status: In Progress → Fix Committed

WRT suggested fix, please note that when comparing 2 float values we need to be consistent eveywhere.
Let's wait until this server bug 882036 is fixed with new generic util methods for comparing floats (via lp:~openerp-dev/openobject-server/trunk-float-rounding-odo) and then we can use that to solve this bug. This should be ready very soon.
Thanks for your patience.

Changed in openobject-addons:
status: Fix Committed → In Progress

Update: the server-side bug 882036 was fixed and a new API is now available for float comparison, including float_round() and a float_compare() methods. It will be appropriate to use float_compare() here, passing the required precision as it comes from the decimal_precision configuration.

Hello Marco Dieckhoff,

We have fixed the problem in lp:~openerp-dev/openobject-addons/trunk-bug-881356-han branch,
It will be merged soon in lp:openobject-addons.

Revision No: 6167
Revision ID: <email address hidden>

Thanks
HAN

Changed in openobject-addons:
status: In Progress → Fix Committed
Numérigraphe (numerigraphe) wrote :

Is there an automatic test associated with this bug? I'd like to try some refactoring but I'd like to make sure I don't introduce this kind of regression.
Lionel.

tags: added: needs-unittest partial-delivery
qdp (OpenERP) (qdp) wrote :

Fix released in trunk version under revision: 6596
revision-id: <email address hidden>

Thanks for the contribution,
Quentin

Changed in openobject-addons:
status: Fix Committed → Fix Released
tags: added: ocb-stock-v1
no longer affects: ocb-addons
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers