[19.0][MIG] stock_account_valuation_report#456
Conversation
bbbf972 to
2281b5a
Compare
There was a problem hiding this comment.
Static review + Functional test on live migrated db with 80k products and 10s of millions of moves. Essentially functionally equivelant despite move from svl's It is still really slow but works as expected, maybe marginally better than before.
Update: OK really slow is understating this, before it was 2 or 3 minutes, now still loading after 15 mins when filter set to Valuation Discrepancy.
Alright effectively unusable on large db's although I can see it works.
Final update: It loaded 80 records using Valuation discrepancy after 1 hour. I scrolled to the next 80 in the hope it had them all. It did not. I feel like the idea of this module has value, but we need to rethink how it is done.
gdgellatly
left a comment
There was a problem hiding this comment.
I am trying to make this more performant, but honestly, so far not much luck, just what I've found so far.
Also I think we canhandle the zero cases better. If there are no aml's and/or no moves we can skip many computations.
| class ProductProduct(models.Model): | ||
| _inherit = "product.product" | ||
|
|
||
| stock_value = fields.Float("Inventory Value", compute="_compute_inventory_value") |
There was a problem hiding this comment.
How does this differ from total_value on product_product? Also why not fields.Monetary
|
I have an alternative implementation on top of this PR that addresses significant performance issues and a bug we encountered: Branch: Bug fixThe existing accounting quantity calculation uses a Fix: replaced with Performance changes
PerformanceOn a database with ~2000 storable products and several million stock moves, the valuation discrepancy filter went from over 60 minutes (original implementation timed out) to ~30 seconds. The remaining ~30s is dominated by core's CaveatThe valuation discrepancy search uses
This means the search filter results should be equivalent for Standard and FIFO, and near-equivalent for AVCO. The accurate value is always shown in the list view itself. |
|
OK I give up, this is almost impossible, the quantity field on aml is used differently in different operations, I just can't get it to make any sense. whether I use the current algorithm (definitely wrong), quantity, or -quantity. Anyway hopefully some of this helps |
|
Hi @gdgellatly, thank you, I am afraid I have not had the time to review your branch. What's the overall performance improvement for your large database? I'll review it and likely merge it to this branch. |
@AaronHForgeFlow It is just a bunch of experiments, and not very well working ones at that. I would not merge it. I actually feel like we might need to go back to the old v7 days where we used to maintain historical aggregates to lower the search window. I will relook later this week to see if we can more easily integrate that approach because it won't be just this module, just this module because of it's non stored computes is super badly affected |
Currently translated at 100.0% (13 of 13 strings) Translation: stock-logistics-reporting-12.0/stock-logistics-reporting-12.0-stock_account_valuation_report Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-reporting-12-0/stock-logistics-reporting-12-0-stock_account_valuation_report/es_MX/
[IMP] stock_account_valuation_report: filter by discrepancies
Currently translated at 100.0% (13 of 13 strings) Translation: stock-logistics-reporting-15.0/stock-logistics-reporting-15.0-stock_account_valuation_report Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-reporting-15-0/stock-logistics-reporting-15-0-stock_account_valuation_report/sl/
…e into account decimal precision
… filtering products with discrepancies also add a filter to see all discrepancies at once
…isplayed with AND conditions
Currently translated at 100.0% (37 of 37 strings) Translation: stock-logistics-reporting-16.0/stock-logistics-reporting-16.0-stock_account_valuation_report Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-reporting-16-0/stock-logistics-reporting-16-0-stock_account_valuation_report/it/
Currently translated at 100.0% (37 of 37 strings) Translation: stock-logistics-reporting-18.0/stock-logistics-reporting-18.0-stock_account_valuation_report Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-reporting-18-0/stock-logistics-reporting-18-0-stock_account_valuation_report/it/
1. stock_value, account_value, valuation_discrepancy: fields.Monetary 2. qty_at_date, account_qty_at_date, qty_discrepancy:added digits 3. Removed precomputed stock_fifo_real_time_aml_ids / stock_move_valuated_ids; AMLs fetched on-demand 4. _search_valuation_discrepancy simplified to float_compare 5. Use aml.parent_state to avoid JOIN 6. Fixed CASE aml.quanitity condition
4fc711b to
2120ec0
Compare
|
Thank you @gdgellatly , I applied the code fixes suggestions. Also partial of the performance improvements suggestions. In a small size database (>10000 products, around (1000 stock moves and journal items) the performance improvement is significant (from 5 seconds to almost immediate). Likely not sufficient for large databases, but I think they are useful for small companies. For large companies, we can store the report and run it overnight with an scheduled action, but I prefer to wait until I have a real use case in v19. |
2120ec0 to
c65b732
Compare
rrebollo
left a comment
There was a problem hiding this comment.
I did some suggestions for your consideration
|
@AaronHForgeFlow would you be so kind to review my #479 in return? |
c65b732 to
41baee6
Compare
…ossible [IMP] stock_account_valuation_report: Domain
41baee6 to
5f0b736
Compare
|
@rrebollo Thank you for the suggestions, I applied them all. |
BhaveshHeliconia
left a comment
There was a problem hiding this comment.
Functional review LGTM!
Hi @AaronHForgeFlow I have not got back to this. But my current thinking is that we create markers to reduce the moves under consideration. You will notice that in certain operation such as changing standard price odoo creates a stock value record. When computing stock value it limits the moves search to the earliest of these records for AVCO/FIFO replay. I have not looked at how to implement yet. We have also traced back most of the performance issues to lot valuated products, so the scope of issues may be limited and I am working with Odoo to see what we can do. |
The stock value info is taken from the stock move instead of the layers