mirror of
https://github.com/inventree/InvenTree.git
synced 2026-09-27 14:16:02 +00:00
Fix floating point comparisons for parameter values (#12927)
- Closes https://github.com/inventree/InvenTree/issues/12923 - Allow tolerant comparisons for floating point values - Additional unit tests
This commit is contained in:
@@ -18,6 +18,9 @@ _UNIT_REG_CACHE_KEY = 'unit_registry_hash'
|
||||
_unit_registry = None
|
||||
_unit_registry_hash: str = ''
|
||||
|
||||
# Relative tolerance used when comparing converted (floating point) numeric values
|
||||
NUMERIC_RELATIVE_TOLERANCE = 1e-9
|
||||
|
||||
logger = structlog.get_logger('inventree')
|
||||
|
||||
# Disable log output for Pint library
|
||||
@@ -371,3 +374,19 @@ def is_dimensionless(value):
|
||||
return True
|
||||
|
||||
return value.to_base_units().units == ureg.dimensionless
|
||||
|
||||
|
||||
def numeric_tolerance(value: float) -> float:
|
||||
"""Return the tolerance to use when comparing the provided numeric value.
|
||||
|
||||
Unit conversion (e.g. '100nF' vs '0.1uF') can produce floating point values which
|
||||
differ in the last few bits, so an exact equality comparison is not reliable.
|
||||
The tolerance scales with the magnitude of the value (a zero value is compared exactly).
|
||||
"""
|
||||
return abs(value) * NUMERIC_RELATIVE_TOLERANCE
|
||||
|
||||
|
||||
def numeric_range(value: float) -> tuple[float, float]:
|
||||
"""Return the (min, max) range within which a numeric value is considered equal."""
|
||||
epsilon = numeric_tolerance(value)
|
||||
return (value - epsilon, value + epsilon)
|
||||
|
||||
@@ -192,19 +192,38 @@ def filter_parameters_by_value(
|
||||
# Some filters are only applicable to string values
|
||||
text_only = any([func in ['icontains'], value_numeric is None])
|
||||
|
||||
# Ensure the function starts with a double underscore
|
||||
if func and not func.startswith('__'):
|
||||
func = f'__{func}'
|
||||
|
||||
# Query for 'numeric' value - this has priority over 'string' value
|
||||
data_numeric = {
|
||||
'parameters_list__template': template,
|
||||
'parameters_list__data_numeric__isnull': False,
|
||||
f'parameters_list__data_numeric{func}': value_numeric,
|
||||
}
|
||||
|
||||
if not text_only:
|
||||
# Numeric values may be the result of unit conversion,
|
||||
# so comparisons must account for floating point error
|
||||
value_min, value_max = InvenTree.conversion.numeric_range(value_numeric)
|
||||
|
||||
match func:
|
||||
case 'gt':
|
||||
data_numeric['parameters_list__data_numeric__gt'] = value_max
|
||||
case 'gte':
|
||||
data_numeric['parameters_list__data_numeric__gte'] = value_min
|
||||
case 'lt':
|
||||
data_numeric['parameters_list__data_numeric__lt'] = value_min
|
||||
case 'lte':
|
||||
data_numeric['parameters_list__data_numeric__lte'] = value_max
|
||||
case _:
|
||||
data_numeric['parameters_list__data_numeric__range'] = (
|
||||
value_min,
|
||||
value_max,
|
||||
)
|
||||
|
||||
query_numeric = Q(**data_numeric)
|
||||
|
||||
# Ensure the function starts with a double underscore
|
||||
if func and not func.startswith('__'):
|
||||
func = f'__{func}'
|
||||
|
||||
# Query for 'string' value
|
||||
data_text = {
|
||||
'parameters_list__template': template,
|
||||
|
||||
@@ -2994,7 +2994,10 @@ class Parameter(
|
||||
|
||||
if self.template.units and self.data_numeric is not None:
|
||||
query = Parameter.objects.filter(
|
||||
template=self.template, data_numeric=self.data_numeric
|
||||
template=self.template,
|
||||
data_numeric__range=InvenTree.conversion.numeric_range(
|
||||
self.data_numeric
|
||||
),
|
||||
)
|
||||
else:
|
||||
query = Parameter.objects.filter(
|
||||
|
||||
@@ -928,6 +928,32 @@ class ParameterAPITests(InvenTreeAPITestCase):
|
||||
param_b.full_clean()
|
||||
param_b.save()
|
||||
|
||||
# SI prefix conversion may introduce floating point error,
|
||||
# but equivalent values must still be detected as duplicates
|
||||
template_cap = common.models.ParameterTemplate.objects.create(
|
||||
name='Capacitance',
|
||||
units='F',
|
||||
description='A globally unique capacitance parameter',
|
||||
unique=common.models.ParameterTemplate.UniqueOptions.GLOBAL,
|
||||
)
|
||||
|
||||
param_c = common.models.Parameter(
|
||||
template=template_cap,
|
||||
model_type=part_a.get_content_type(),
|
||||
model_id=part_a.pk,
|
||||
data='100nF',
|
||||
)
|
||||
param_c.full_clean()
|
||||
param_c.save()
|
||||
|
||||
with self.assertRaises(ValidationError):
|
||||
common.models.Parameter(
|
||||
template=template_cap,
|
||||
model_type=part_b.get_content_type(),
|
||||
model_id=part_b.pk,
|
||||
data='0.1uF',
|
||||
).full_clean()
|
||||
|
||||
def test_copy_unique_parameters(self):
|
||||
"""Test that 'unique' parameters are skipped when copying parameters between model instances."""
|
||||
from part.models import Part
|
||||
|
||||
@@ -688,6 +688,49 @@ class ParameterFilterTest(InvenTreeAPITestCase):
|
||||
|
||||
self.assertEqual(len(response), 5)
|
||||
|
||||
def test_filter_si_prefix(self):
|
||||
"""Test filtering by values which differ only in SI prefix.
|
||||
|
||||
Unit conversion introduces floating point error (e.g. '100nF' != '0.1uF'),
|
||||
so numeric comparisons must be tolerant of this.
|
||||
"""
|
||||
template = ParameterTemplate.objects.create(
|
||||
name='Capacitance', description='Capacitance of the part', units='F'
|
||||
)
|
||||
|
||||
values = ['10nF', '100nF', '0.1uF', '1uF', '1000nF']
|
||||
parts = list(Part.objects.all()[: len(values)])
|
||||
|
||||
for part, value in zip(parts, values, strict=True):
|
||||
Parameter.objects.create(content_object=part, template=template, data=value)
|
||||
|
||||
filters = [
|
||||
('', '100n', 2),
|
||||
('', '100nF', 2),
|
||||
('', '0.1u', 2),
|
||||
('', '.1uF', 2),
|
||||
('', '100000pF', 2),
|
||||
('', '1uF', 2),
|
||||
('', '1000n', 2),
|
||||
('', '0.01u', 1),
|
||||
('', '101nF', 0),
|
||||
('_ne', '0.1uF', 48),
|
||||
('_gt', '0.1uF', 2),
|
||||
('_gte', '100nF', 4),
|
||||
('_lt', '100nF', 1),
|
||||
('_lte', '0.1uF', 3),
|
||||
('_gt', '1000nF', 0),
|
||||
('_gte', '1uF', 2),
|
||||
('_lt', '1uF', 3),
|
||||
('_lte', '1000nF', 5),
|
||||
]
|
||||
|
||||
for operator, value, expected_count in filters:
|
||||
filter_name = f'parameter_{template.pk}' + operator
|
||||
response = self.get(self.url, {filter_name: value}, expected_code=200).data
|
||||
|
||||
self.assertEqual(len(response), expected_count, f'{filter_name}={value}')
|
||||
|
||||
def test_filter_multiple(self):
|
||||
"""Test filtering by multiple parameters."""
|
||||
data = {f'parameter_{self.template_length.pk}_lt': '225'}
|
||||
|
||||
Reference in New Issue
Block a user