[FIX] product: generate variant combinations faster
The function `_get_possible_combinations` generates possibles combinations for the product template attribute values (ptav) of each product template attribute line (ptal) of a given product template. It used to iterate over them using a cartesian product. A combination can be invalid if it contains two incompatibles ptav. The problem is that it used to filter out invalid combinations after generating them. It's a problem because there can be a lot of invalid combinations to filter out before finding a valid combination. Even before the first valid combination if the first ptav of the two first ptal are incompatible. The solution brought by this commit is to reject the invalid combinations while building them rather than after. It does so by using a cartesian product implementation with early rejection. The algorithm tests each ptav when incorporating it in a partial combination and if it's incompatible, it goes to the next partial combination and thus skips all combinations starting with the invalid partial combination. The client that reported the issue has a product template with about 25 ptal with an average of 9 ptav. The ptav of the first ptal was incompatible with the first ptav of second ptal. It had to build and filter out 5.76*10^15 ptav before finding the first valid ptav. If we generate 10^6 ptav by second (it's much less) user would get first ptav after 182 years. Now it's instantaneous. opw-2335936 closes odoo/odoo#65809 X-original-commit: 98616c4676aa6ef5507110c0e407a640082abbaf Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>
This commit is contained in:
Adrien Horgnies
@@ -3,6 +3,7 @@
|
||||
|
||||
import itertools
|
||||
import logging
|
||||
from collections import defaultdict
|
||||
|
||||
from odoo import api, fields, models, tools, _, SUPERUSER_ID
|
||||
from odoo.exceptions import ValidationError, RedirectWarning, UserError
|
||||
@@ -1003,6 +1004,92 @@ class ProductTemplate(models.Model):
|
||||
"""
|
||||
return next(self._get_possible_combinations(parent_combination, necessary_values), self.env['product.template.attribute.value'])
|
||||
|
||||
def _cartesian_product(self, product_template_attribute_values_per_line, parent_combination):
|
||||
"""
|
||||
Generate all possible combination for attributes values (aka cartesian product).
|
||||
It is equivalent to itertools.product except it skips invalid partial combinations before they are complete.
|
||||
|
||||
Imagine the cartesian product of 'A', 'CD' and range(1_000_000) and let's say that 'A' and 'C' are incompatible.
|
||||
If you use itertools.product or any normal cartesian product, you'll need to filter out of the final result
|
||||
the 1_000_000 combinations that start with 'A' and 'C' . Instead, This implementation will test if 'A' and 'C' are
|
||||
compatible before even considering range(1_000_000), skip it and and continue with combinations that start
|
||||
with 'A' and 'D'.
|
||||
|
||||
It's necessary for performance reason because filtering out invalid combinations from standard Cartesian product
|
||||
can be extremely slow
|
||||
|
||||
:param product_template_attribute_values_per_line: the values we want all the possibles combinations of.
|
||||
One list of values by attribute line
|
||||
:return: a generator of product template attribute value
|
||||
"""
|
||||
if not product_template_attribute_values_per_line:
|
||||
return
|
||||
|
||||
all_exclusions = {self.env['product.template.attribute.value'].browse(k):
|
||||
self.env['product.template.attribute.value'].browse(v) for k, v in
|
||||
self._get_own_attribute_exclusions().items()}
|
||||
# The following dict uses product template attribute values as keys
|
||||
# 0 means the value is acceptable, greater than 0 means it's rejected, it cannot be negative
|
||||
# Bear in mind that several values can reject the same value and the latter can only be included in the
|
||||
# considered combination if no value rejects it.
|
||||
# This dictionary counts how many times each value is rejected.
|
||||
# Each time a value is included in the considered combination, the values it rejects are incremented
|
||||
# When a value is discarded from the considered combination, the values it rejects are decremented
|
||||
current_exclusions = defaultdict(int)
|
||||
for exclusion in self._get_parent_attribute_exclusions(parent_combination):
|
||||
current_exclusions[self.env['product.template.attribute.value'].browse(exclusion)] += 1
|
||||
partial_combination = self.env['product.template.attribute.value']
|
||||
|
||||
# The following list reflects product_template_attribute_values_per_line
|
||||
# For each line, instead of a list of values, it contains the index of the selected value
|
||||
# -1 means no value has been picked for the line in the current (partial) combination
|
||||
value_index_per_line = [-1] * len(product_template_attribute_values_per_line)
|
||||
# determines which line line we're working on
|
||||
line_index = 0
|
||||
|
||||
while True:
|
||||
current_line_values = product_template_attribute_values_per_line[line_index]
|
||||
current_ptav_index = value_index_per_line[line_index]
|
||||
current_ptav = current_line_values[current_ptav_index]
|
||||
|
||||
# removing exclusions from current_ptav as we're removing it from partial_combination
|
||||
if current_ptav_index >= 0:
|
||||
for ptav_to_include_back in all_exclusions[current_ptav]:
|
||||
current_exclusions[ptav_to_include_back] -= 1
|
||||
partial_combination -= current_ptav
|
||||
|
||||
if current_ptav_index < len(current_line_values) - 1:
|
||||
# go to next value of current line
|
||||
value_index_per_line[line_index] += 1
|
||||
current_line_values = product_template_attribute_values_per_line[line_index]
|
||||
current_ptav_index = value_index_per_line[line_index]
|
||||
current_ptav = current_line_values[current_ptav_index]
|
||||
elif line_index != 0:
|
||||
# reset current line, and then go to previous line
|
||||
value_index_per_line[line_index] = - 1
|
||||
line_index -= 1
|
||||
continue
|
||||
else:
|
||||
# we're done if we must reset first line
|
||||
break
|
||||
|
||||
# adding exclusions from current_ptav as we're incorporating it in partial_combination
|
||||
for ptav_to_exclude in all_exclusions[current_ptav]:
|
||||
current_exclusions[ptav_to_exclude] += 1
|
||||
partial_combination += current_ptav
|
||||
|
||||
# test if included values excludes current value or if current value exclude included values
|
||||
if current_exclusions[current_ptav] or \
|
||||
any(intersection in partial_combination for intersection in all_exclusions[current_ptav]):
|
||||
continue
|
||||
|
||||
if line_index == len(product_template_attribute_values_per_line) - 1:
|
||||
# submit combination if we're on the last line
|
||||
yield partial_combination
|
||||
else:
|
||||
# else we go to the next line
|
||||
line_index += 1
|
||||
|
||||
def _get_possible_combinations(self, parent_combination=None, necessary_values=None):
|
||||
"""Generator returning combinations that are possible, following the
|
||||
sequence of attributes and values.
|
||||
@@ -1030,24 +1117,21 @@ class ProductTemplate(models.Model):
|
||||
return _("The product template is archived so no combination is possible.")
|
||||
|
||||
necessary_values = necessary_values or self.env['product.template.attribute.value']
|
||||
necessary_attributes = necessary_values.mapped('attribute_id')
|
||||
ptal_stack = [self.valid_product_template_attribute_line_ids.filtered(lambda ptal: ptal.attribute_id not in necessary_attributes)]
|
||||
combination_stack = [necessary_values]
|
||||
necessary_attribute_lines = necessary_values.mapped('attribute_line_id')
|
||||
attribute_lines = self.valid_product_template_attribute_line_ids.filtered(lambda ptal: ptal not in necessary_attribute_lines)
|
||||
|
||||
# keep going while we have attribute lines to test
|
||||
while len(ptal_stack):
|
||||
attribute_lines = ptal_stack.pop()
|
||||
combination = combination_stack.pop()
|
||||
if not attribute_lines and self._is_combination_possible(necessary_values, parent_combination):
|
||||
yield necessary_values
|
||||
|
||||
if not attribute_lines:
|
||||
# full combination, if it's possible return it, otherwise skip it
|
||||
if self._is_combination_possible(combination, parent_combination):
|
||||
yield(combination)
|
||||
else:
|
||||
# we have remaining attribute lines to consider
|
||||
for ptav in reversed(attribute_lines[0].product_template_value_ids._only_active()):
|
||||
ptal_stack.append(attribute_lines[1:])
|
||||
combination_stack.append(combination + ptav)
|
||||
product_template_attribute_values_per_line = [
|
||||
ptal.product_template_value_ids
|
||||
for ptal in attribute_lines
|
||||
]
|
||||
|
||||
for partial_combination in self._cartesian_product(product_template_attribute_values_per_line, parent_combination):
|
||||
combination = partial_combination + necessary_values
|
||||
if self._is_combination_possible(combination, parent_combination):
|
||||
yield combination
|
||||
|
||||
return _("There are no remaining possible combination.")
|
||||
|
||||
|
||||
@@ -393,6 +393,107 @@ class TestProductAttributeValueConfig(TestProductAttributeValueCommon):
|
||||
gen = self.computer._get_possible_combinations()
|
||||
self.assertIsNone(next(gen, None))
|
||||
|
||||
# Testing parent case
|
||||
mouse = self.env['product.template'].create({'name': 'Mouse'})
|
||||
self.assertTrue(mouse._is_combination_possible(self.env['product.template.attribute.value']))
|
||||
|
||||
# prep work for the last part of the test
|
||||
color_attribute = self.env['product.attribute'].create({'name': 'Color'})
|
||||
color_red = self.env['product.attribute.value'].create({
|
||||
'name': 'Red',
|
||||
'attribute_id': color_attribute.id,
|
||||
})
|
||||
color_green = self.env['product.attribute.value'].create({
|
||||
'name': 'Green',
|
||||
'attribute_id': color_attribute.id,
|
||||
})
|
||||
self.env['product.template.attribute.line'].create({
|
||||
'product_tmpl_id': mouse.id,
|
||||
'attribute_id': color_attribute.id,
|
||||
'value_ids': [(6, 0, [color_red.id, color_green.id])],
|
||||
})
|
||||
|
||||
mouse_color_red = self._get_product_template_attribute_value(color_red, mouse)
|
||||
mouse_color_green = self._get_product_template_attribute_value(color_green, mouse)
|
||||
|
||||
self._add_exclude(computer_ssd_256, mouse_color_red, mouse)
|
||||
self.assertEqual(mouse._get_first_possible_combination(parent_combination=computer_ssd_256 + computer_ram_8 + computer_hdd_1), mouse_color_green)
|
||||
|
||||
# Test to see if several attribute_line for same attribute is well handled
|
||||
color_blue = self.env['product.attribute.value'].create({
|
||||
'name': 'Blue',
|
||||
'attribute_id': color_attribute.id,
|
||||
})
|
||||
color_yellow = self.env['product.attribute.value'].create({
|
||||
'name': 'Yellow',
|
||||
'attribute_id': color_attribute.id,
|
||||
})
|
||||
self.env['product.template.attribute.line'].create({
|
||||
'product_tmpl_id': mouse.id,
|
||||
'attribute_id': color_attribute.id,
|
||||
'value_ids': [(6, 0, [color_blue.id, color_yellow.id])],
|
||||
})
|
||||
mouse_color_yellow = self._get_product_template_attribute_value(color_yellow, mouse)
|
||||
self.assertEqual(mouse._get_first_possible_combination(necessary_values=mouse_color_yellow), mouse_color_red + mouse_color_yellow)
|
||||
|
||||
# Making sure it's not extremely slow (has to discard invalid combinations early !)
|
||||
product_template = self.env['product.template'].create({
|
||||
'name': 'many combinations',
|
||||
})
|
||||
|
||||
for i in range(10):
|
||||
# create the attributes
|
||||
product_attribute = self.env['product.attribute'].create({
|
||||
'name': "att %s" % i,
|
||||
'create_variant': 'dynamic',
|
||||
'sequence': i,
|
||||
})
|
||||
|
||||
for j in range(50):
|
||||
# create the attribute values
|
||||
value = self.env['product.attribute.value'].create([{
|
||||
'name': "val %s" % j,
|
||||
'attribute_id': product_attribute.id,
|
||||
'sequence': j,
|
||||
}])
|
||||
|
||||
# set attribute and attribute values on the template
|
||||
self.env['product.template.attribute.line'].create([{
|
||||
'attribute_id': product_attribute.id,
|
||||
'product_tmpl_id': product_template.id,
|
||||
'value_ids': [(6, 0, product_attribute.value_ids.ids)]
|
||||
}])
|
||||
|
||||
self._add_exclude(
|
||||
self._get_product_template_attribute_value(product_template.attribute_line_ids[1].value_ids[0],
|
||||
model=product_template),
|
||||
self._get_product_template_attribute_value(product_template.attribute_line_ids[0].value_ids[0],
|
||||
model=product_template),
|
||||
product_template)
|
||||
self._add_exclude(
|
||||
self._get_product_template_attribute_value(product_template.attribute_line_ids[0].value_ids[0],
|
||||
model=product_template),
|
||||
self._get_product_template_attribute_value(product_template.attribute_line_ids[1].value_ids[1],
|
||||
model=product_template),
|
||||
product_template)
|
||||
|
||||
combination = self.env['product.template.attribute.value']
|
||||
for idx, ptal in enumerate(product_template.attribute_line_ids):
|
||||
if idx != 1:
|
||||
value = ptal.product_template_value_ids[0]
|
||||
else:
|
||||
value = ptal.product_template_value_ids[2]
|
||||
combination += value
|
||||
|
||||
started_at = time.time()
|
||||
self.assertEqual(product_template._get_first_possible_combination(), combination)
|
||||
elapsed = time.time() - started_at
|
||||
# It should be about instantaneous, 0.5 to avoid false positives
|
||||
self.assertLess(elapsed, 0.5)
|
||||
|
||||
|
||||
|
||||
|
||||
def test_get_closest_possible_combinations(self):
|
||||
computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256)
|
||||
computer_ssd_512 = self._get_product_template_attribute_value(self.ssd_512)
|
||||
|
||||
Reference in New Issue
Block a user