Floating Point / Rounding issue in stock partial picking wizard

[Marco Dieckhoff]

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.

[Ferdinand (office-chricar)]

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)]

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.

[Lorenzo Battistini (elbati)]

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

[Olivier Dony (Odoo) (odo-openerp)]

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

[Hardik Ansodariya (OpenERP) (han-tinyerp)]

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

[Olivier Dony (Odoo) (odo-openerp)]

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.

[Olivier Dony (Odoo) (odo-openerp)]

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.

[Hardik Ansodariya (OpenERP) (han-tinyerp)]

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

[Numérigraphe (numerigraphe)]

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.

[qdp (OpenERP) (qdp)]

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

Thanks for the contribution,
Quentin