stock value rounding differences

Bug #372045 reported by Ferdinand
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Invalid
Wishlist
Unassigned

Bug Description

the current calculation method for stock value and accounting are prone to show rounding differences

from stock.py product_detail etc

Stock value = sum(stock_move.product_qty) * standard_price

this will be greatly improved by using standard_price_coeff because of more decimal digits
see
https://bugs.launchpad.net/openobject-addons/+bug/358584

if a product is storen in more than ONE location
we will have
Stock value(for each location) = sum(stock_move.product_qty, location) * standard_price
which will not necessary equal
Stock value = sum(stock_move.product_qty) * standard_price
and not equal
sum(amount) in account_move

IMHO it is mandatory to store the amount in stock_move too to avoid this problem.
It should not be a performance problem as stock_moves have to be read for sum(qty) anyhow

please give me advice on how to implement this once I have added amount and price_unit_id to stock_move

Related branches

Revision history for this message
Ferdinand (office-chricar) wrote :

thinking of this I suggest
* to add value and
* replace price unit value with a function (value/quantity)

Changed in openobject-addons:
assignee: nobody → Harry (Open ERP) (hmo-tinyerp)
Revision history for this message
Ferdinand (office-chricar) wrote :
Revision history for this message
Jay Vora (Serpent Consulting Services) (jayvora) wrote :

Hello Ferdinand,

Would you please provide us the exact lines of change?

So, it would be better to fix on stable.

Thanks.

Revision history for this message
Ferdinand (office-chricar) wrote :

the problem is that hda disapproved the chricar_price_unit - I mailed him personaly but got no response

the stock evaluation, introduction of price_unit, transfer of values into the finance accounts and layout improvements is a major change and I agreed with fabien to put all this into this branch and we should discuss how to integrate it into trunk/5.2

all code is found (mostly with comments FIXME) in the modules stock, purchase, sale and dependant - and needs some polish and some mainly usablility) enhancements (which are beyond my technical knowledge)

we use the branch https://code.launchpad.net/~openerp-commiter/openobject-addons/chricar_price_unit since 6 month now in production.

Revision history for this message
Ferdinand (office-chricar) wrote :

I hope that I will have time AFTER we have discussed these matters during community/partner days.
It should be possible to provide small patches per module, although some issues touch several modules.

Changed in openobject-addons:
status: New → Confirmed
Changed in openobject-addons:
assignee: Harry (Open ERP) (hmo-tinyerp) → nobody
Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Hello Ferdinand,

I'm marking this bug as incomplete + wishlist, in order to keep it more up-to-date.

We will need more precise information to be able to understand what you propose exactly (as you know, we can't directly merge your price_unit branch).

If possible, please post a small patch (a few lines) that fixes the problem you have identified, or explain more precisely what lines in the code are incorrect (in trunk).

But please keep in mind the following:
 - for 5.0 we can't consider this, as it would be a wishlist/improvement, so please only look at trunk
 - for trunk the code is different from what you describe, so you may want to look at the _get_accounting_values() method in stock/stock.py
 - in all case, valuation must be handled separately for each move in a picking (because we can do partial pickings processing), so we cannot sum together several moves' quantities
 - the current code of _get_accounting_values() seems to handle things properly, including when products in one move are picked from multiple locations

Thanks!

Changed in openobject-addons:
importance: Undecided → Wishlist
status: Confirmed → Incomplete
Revision history for this message
Ferdinand (office-chricar) wrote :

let's forget V5 for this....

please see an issue just happened
https://bugs.launchpad.net/openobject-addons/+bug/636899

in trunk stock.py I see
1894 if move.product_id.cost_method == 'average' and move.price_unit:
1895 amount = q * move.price_unit

I strongly suggest to store this amount - I called it
move_value_cost (and move_value_sales ) and would be happy if you could take this term as agreed on the accounting expert list.

and calculate move.price_unit = move_value_cost / q or store it too ( it's cheap)
it does not make much sense (performance) to do a q * move.price_unit every time OpenERP reads stock_moves and this is done every time OpenERP opens a product.

the move_value_cost is exactly the amount which will be used directly or as aggregate in the accounting.

and we must make sure that stock_accounting values match exactly accounting values.

and sum(q * move.price_unit) is likely to be different from sum(move_value_cost) especially if this is done once in python and once in psql and for summing up some 100.000 records I hope we will use psql.

hope this clarifies the issue.

Changed in openobject-addons:
status: Incomplete → New
Revision history for this message
Ferdinand (office-chricar) wrote :

Sorry I am a thinking one step further - "stock accounting" which is missing in V5 (and trunk)

product_product has (still) no value which matches qty_available

it is absolutely mandatory to have lists and reports which show
product / qty_available / value_qty_available (for selected periods)

value_qty_available was (is) calculated in reports as qty_available * product_product.(...price..) which is absolutely useless/wrong for past periods if the price changed for what reason so ever.

and these aggregate value_qty_available must match financial accounts.

so I also suggest to move all necessary value calculation from reports to function fields of the object.
(see my remarks and suggestions in price_unit branch for account sums )

Revision history for this message
Harry (OpenERP) (hmo-tinyerp) wrote :

Hello Dr. Ferdinand,

I would like to prefer one module for this nice feature. We have made this module product_price_unit base on your work in this branch: https://code.launchpad.net/~openerp-dev/openobject-addons/product_price_unit_trunk.

Thanks

Revision history for this message
Ferdinand (office-chricar) wrote :

my recent posts were triggered by by 2 facts
1)
https://bugs.launchpad.net/openobject-addons/+bug/636899
as long we do not take precautions it will not be possible to reliable filter products with
* qty_available "equal to 0"
* qty_available "not equal to 0"
due to rounding errors.

for example
in one company reaching qty_available = 0 triggers to count the physical inventory.

2)
for incoming products many companies will use average price.
But the current average price (which is or at least was stored in product_product) can't be taken to evaluated evaluate past (years end) qty_available - after awhile this will never match accounting figures.

I just strongly oppose to set up the base system in a way that we have to add "nice" modules to make it work properly.

may be this issue should be discussed in the expert list.

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

@Harry:

I think you are talking about a different issue: the main "price_unit" feature that is implemented in the chricar branch is not what this bug is about (even if the chricar branch contains code related to this bug, because it is a mix of many different things)

@Ferdinand,

Thank you for your answers, but unfortunately I still fail to see any bug here. To address more specifically your points:

Point 1: Is this the correct bug reference? This one is about base_contact, does not seem related to stock valuation.
Please explain why qty_available is related in any way to inventory valuation (which this bug is about)?

Point 2: Weighted average price is implemented in OpenERP as a way to evaluate the average cost/value of products you currently have in stock. It is stored on the product and is not meant for accounting reports, nor to evaluate past years. It is also not used to do historical analysis (nothing to do with periods, etc.)! Of course if you still have old products in your warehouse their value is still partially taken into account in the average price.
This average can be used correctly to compute: (current_product_qty * current_product_average_cost) as the current value of the total inventory for that product. This is always based on average cost, we are not considering every individual product.
This method is applicable mostly for items that are all equivalent regardless of their age.
I'm afraid anything else is out of the scope of the core addons, at least for v6.

Comment #7 : You mention line 1894 of stock.py, and I really don't understand your point. This stock_move.price_unit value is really only used temporarily to record the current product cost to use when writing the accounting entries corresponding to the stock.move (if and only if real-time stock valuation is enabled).
This is very different from updating the average product cost on the product. We are talking of different things here: real-time inventory valuation in Financial Journals (with normal accounting history) versus determination of current average product cost (no history at all). And please explain where we are doing 100.000 sums of these values, as I said it's not used for anything apart writing one Journal Entry!

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Ferdinand, here are some more precisions:

Basically I think we are discussing different kinds of valuations, and we should not mix them :-)

You are right when you say that using the current average cost of the product for computing the "value_qty_available" of past periods is not accurate. And if this is the case this should be another bug report I think.
Now from what I understand you would like to replace the average cost with a function field that computes it based on a set of stock.moves. However this cannot work currently as you are free to update the average price manually if you want, and no stock.move will correspond to that. And in fact it does not matter because we should not use the average price for anything else than knowing the current average cost of a product.
We could discuss changing this but not for v6.0, we really need to finish making it stable.

Also please note that in v6 doing a manual change of the average cost is done via a wizard that can post the corresponding Journal Entires to make sure that the GL accounts are indeed correctly representing the inventory value. So if indeed you have enabled the real-time inventory valuation, the numbers will be correct in accounting, and you have history there if you need it.

As a summary:

A. Let's not try to fit the current average product cost of a product into any kind of historical reporting at the moment, as it is not meant for it. If you need that, please use accounting entries/reports exclusively, for example through real-time inventory valuation.
And if we do have reports that use the average cost incorrectly, it's probably only to provide some estimation, and it might be better to remove this value completely.

B. Another quite important point: no, the values in accounting reports will not always match the current values displayed in the product list or in warehouse reports, because they are many factors that can make them diverge. And it's not a bug because they are not meant to be the same (at least for the moment).

I hope this explanation is clear.

If you still think there is a precision issue or error in computing either the *current* average cost *or* the GL journal entries that are posted due to real-time inventory valuation, please provide a precise explanation or patch (I'm currently working on some error cases at the moment in v6)
Otherwise please close this bug (pure housekeeping) and let's discuss your suggestions in a blueprint or another wishlist bug for a later version :-)

Revision history for this message
Ferdinand (office-chricar) wrote :

I agree we should not mix the issues, it's a complex issue, but still remains valid.

* Warehouse / Location Structure / Products by Location requests from - to dates

in stock/report/report_stock_move.py
I see
* line 94 >> pt.standard_price * sum(sm.product_qty) as value,
* line 156 >> sum(-pt.standard_price * m.product_qty * u.factor)::decimal(16,2) as value

which multiplies product template standard price * move qty which is wrong for the past and will change depending what price is currently available.
I still do not think this is enterprise ready and will cause a lot of headache if people start asking why OpenEPR produces different "values" for the same period.
And I still believe that stock values should match accounting values to comply to simplest accounting rules.
IMHO this (A+B) in https://bugs.launchpad.net/openobject-addons/+bug/372045/comments/12) is a conceptual mistake.
No auditor will accept that physical inventory value does not match accounting values.

Neither I see any need for evaluation of past stock quantities with current average price for governing a company except for devaluation of stock or to see hidden reserves. but that is really a completely different issue and needs again the correct past evaluation.

BTW the 2 formulas risk not to produce the same result for mathematical, psql and pyhtonic reasons due to float of product_qty

If OpenERP would use (m.price_unit * m.product_qty) we would get consistent number
and instead of (expensive) calculation of (m.price_unit * m.product_qty) we should store this value once as it will be needed all the time.

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Hello Ferdinand,

Sorry if I sound like a broken record:

As explained, we don't want to mix real-time accounting entries for inventory valuation (ok for historical analysis) and the instantaneous estimation of warehouse contents value based on the current product cost (which is not meant for historical analysis at all).
Anything else is out of scope for v6.0.

Seeing that the description of this bug is still invalid, I will mark the bug so.
The best we could do at the moment is to change it to "Opinion+Wishlist" if you properly update the description and title to reflect your point. Otherwise this is probably best discussed on a linked blueprint, to be reviewed after v6.0.

Thanks for bringing up this topic!

Changed in openobject-addons:
status: New → Invalid
Revision history for this message
Ferdinand (office-chricar) wrote :

I give up

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.