From 5ebd89e97f360991addb63eb134a7f440f3da672 Mon Sep 17 00:00:00 2001 From: BenYuan Date: Fri, 25 Oct 2024 17:47:13 +0000 Subject: [PATCH 1/4] [SCFD-3113] Added validation that vector should not contain None components --- flow360/component/simulation/unit_system.py | 75 +++++++++++++------ .../framework/test_unit_system_v2.py | 11 +++ 2 files changed, 62 insertions(+), 24 deletions(-) diff --git a/flow360/component/simulation/unit_system.py b/flow360/component/simulation/unit_system.py index e18d85b63..d7a111699 100644 --- a/flow360/component/simulation/unit_system.py +++ b/flow360/component/simulation/unit_system.py @@ -157,25 +157,39 @@ def _has_dimensions(quant, dim): return arg_dim == dim -def _unit_object_parser(value, unyt_types: List[type]): +def _unit_object_parser(value, unyt_types: List[type], **kwargs): """ Parses {'value': value, 'units': units}, into unyt_type object : unyt.unyt_quantity, unyt.unyt_array """ - if isinstance(value, dict) and "units" in value: - if "value" in value: - for unyt_type in unyt_types: - try: - return unyt_type(value["value"], value["units"], dtype=np.float64) - except u.exceptions.UnitParseError: - pass - except RuntimeError: - pass - except KeyError: - pass - else: - raise TypeError( - f"Dimensioned type instance {value} expects a 'value' field which was not given" - ) + if isinstance(value, dict) is False or "units" not in value: + return value + if "value" not in value: + raise TypeError( + f"Dimensioned type instance {value} expects a 'value' field which was not given" + ) + for unyt_type in unyt_types: + try: + dimensioned_value = unyt_type(value["value"], value["units"], dtype=np.float64) + # Check for Nan. This usually is a result of None being in the input value. + # with dtype=np.float64, None will be converted to np.nan. + # We need (IIRC) np.float64 to ensure consistent hashing + if kwargs.get("allow_inf_nan", False) is False: + if np.ndim(dimensioned_value.value) == 0 and np.isnan(dimensioned_value.value): + raise ValueError("NaN or None found in input scalar which is not allowed.") + if np.ndim(dimensioned_value.value) > 0 and any(np.isnan(dimensioned_value.value)): + raise ValueError("NaN or None found in input array which is not allowed.") + return dimensioned_value + except u.exceptions.UnitParseError: + pass + except RuntimeError as e: + if str(e) == "unyt_quantity values must be numeric": + # Have to catch here otherwise there will + # be "No class found for unit_name" which is confusing. + raise ValueError( + "The numerical part of the input dimensioned value is incorrect." + ) from e + except KeyError: + pass return value @@ -285,13 +299,16 @@ class _DimensionedType(metaclass=ABCMeta): has_defaults = True @classmethod - def validate(cls, value): + # pylint: disable=unused-argument + def validate(cls, value, *args, **kwargs): """ Validator for value """ try: - value = _unit_object_parser(value, [u.unyt_quantity, _Flow360BaseUnit.factory]) + value = _unit_object_parser( + value, [u.unyt_quantity, _Flow360BaseUnit.factory], **kwargs + ) value = _is_unit_validator(value) if cls.has_defaults: value = _unit_inference_validator(value, cls.dim_name) @@ -358,12 +375,18 @@ def get_class_object(cls, dim_type, **kwargs): """Get a dynamically created metaclass representing the constraint""" class _ConType(pd.BaseModel): - value: Annotated[float, annotated_types.Interval(**kwargs)] + kwargs.pop("allow_inf_nan", None) + value: Annotated[ + float, + annotated_types.Interval( + **{k: v for k, v in kwargs.items() if k != "allow_inf_nan"} + ), + ] - def validate(con_cls, value, *args): + def validate(con_cls, value, *args, **kwargs): """Additional validator for value""" try: - dimensioned_value = dim_type.validate(value) + dimensioned_value = dim_type.validate(value, kwargs) # Workaround to run annotated validation for numeric value of field _ = _ConType(value=dimensioned_value.value) @@ -411,7 +434,9 @@ def Constrained(cls, gt=None, ge=None, lt=None, le=None, allow_inf_nan=False): """ Utility method to generate a dimensioned type with constraints based on the pydantic confloat """ - return cls._Constrained.get_class_object(cls, gt=gt, ge=ge, lt=lt, le=le) + return cls._Constrained.get_class_object( + cls, gt=gt, ge=ge, lt=lt, le=le, allow_inf_nan=allow_inf_nan + ) # pylint: disable=invalid-name @classproperty @@ -472,10 +497,12 @@ def __get_pydantic_json_schema__( return schema - def validate(vec_cls, value, *args): + def validate(vec_cls, value, *args, **kwargs): """additional validator for value""" try: - value = _unit_object_parser(value, [u.unyt_array, _Flow360BaseUnit.factory]) + value = _unit_object_parser( + value, [u.unyt_array, _Flow360BaseUnit.factory], **kwargs + ) value = _is_unit_validator(value) is_collection = isinstance(value, Collection) or ( diff --git a/tests/simulation/framework/test_unit_system_v2.py b/tests/simulation/framework/test_unit_system_v2.py index bfdaefea5..852bd54fc 100644 --- a/tests/simulation/framework/test_unit_system_v2.py +++ b/tests/simulation/framework/test_unit_system_v2.py @@ -491,6 +491,17 @@ def test_unit_system(): omega={"value": [1, 1, 1], "units": "rad/s"}, ) + with pytest.raises( + pd.ValidationError, + match=r"Value error, NaN or None found in input array which is not allowed.", + ): + data = VectorDataWithUnits( + pt=None, + vec={"value": [1, 1, None], "units": "N"}, + ax={"value": [0, 0, 1], "units": "m"}, + omega={"value": [1, 1, 1], "units": "rad/s"}, + ) + with u.SI_unit_system: # Note that for union types the first element of union that passes validation is inferred! data = VectorDataWithUnits( From 814f09dad7a40b61bca5542b01870d1511a10e50 Mon Sep 17 00:00:00 2001 From: BenYuan Date: Fri, 8 Nov 2024 18:54:31 +0000 Subject: [PATCH 2/4] Moved the implementation into a dedicated validator --- flow360/component/simulation/unit_system.py | 40 ++++++++----------- .../framework/test_unit_system_v2.py | 3 +- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/flow360/component/simulation/unit_system.py b/flow360/component/simulation/unit_system.py index d7a111699..18b4bb31a 100644 --- a/flow360/component/simulation/unit_system.py +++ b/flow360/component/simulation/unit_system.py @@ -157,7 +157,7 @@ def _has_dimensions(quant, dim): return arg_dim == dim -def _unit_object_parser(value, unyt_types: List[type], **kwargs): +def _unit_object_parser(value, unyt_types: List[type]): """ Parses {'value': value, 'units': units}, into unyt_type object : unyt.unyt_quantity, unyt.unyt_array """ @@ -169,25 +169,11 @@ def _unit_object_parser(value, unyt_types: List[type], **kwargs): ) for unyt_type in unyt_types: try: - dimensioned_value = unyt_type(value["value"], value["units"], dtype=np.float64) - # Check for Nan. This usually is a result of None being in the input value. - # with dtype=np.float64, None will be converted to np.nan. - # We need (IIRC) np.float64 to ensure consistent hashing - if kwargs.get("allow_inf_nan", False) is False: - if np.ndim(dimensioned_value.value) == 0 and np.isnan(dimensioned_value.value): - raise ValueError("NaN or None found in input scalar which is not allowed.") - if np.ndim(dimensioned_value.value) > 0 and any(np.isnan(dimensioned_value.value)): - raise ValueError("NaN or None found in input array which is not allowed.") - return dimensioned_value + return unyt_type(value["value"], value["units"], dtype=np.float64) except u.exceptions.UnitParseError: pass - except RuntimeError as e: - if str(e) == "unyt_quantity values must be numeric": - # Have to catch here otherwise there will - # be "No class found for unit_name" which is confusing. - raise ValueError( - "The numerical part of the input dimensioned value is incorrect." - ) from e + except RuntimeError: + pass except KeyError: pass return value @@ -268,6 +254,12 @@ def _has_dimensions_validator(value, dim): return value +def _nan_inf_vector_validator(value): + if np.ndim(value.value) > 0 and (any(np.isnan(value.value)) or any(np.isinf(value.value))): + raise ValueError("NaN/Inf/None found in input array. Please ensure your input is complete.") + return value + + def _enforce_float64(unyt_obj): """ This make sure all the values are float64 to minimize floating point errors @@ -306,9 +298,7 @@ def validate(cls, value, *args, **kwargs): """ try: - value = _unit_object_parser( - value, [u.unyt_quantity, _Flow360BaseUnit.factory], **kwargs - ) + value = _unit_object_parser(value, [u.unyt_quantity, _Flow360BaseUnit.factory]) value = _is_unit_validator(value) if cls.has_defaults: value = _unit_inference_validator(value, cls.dim_name) @@ -500,9 +490,7 @@ def __get_pydantic_json_schema__( def validate(vec_cls, value, *args, **kwargs): """additional validator for value""" try: - value = _unit_object_parser( - value, [u.unyt_array, _Flow360BaseUnit.factory], **kwargs - ) + value = _unit_object_parser(value, [u.unyt_array, _Flow360BaseUnit.factory]) value = _is_unit_validator(value) is_collection = isinstance(value, Collection) or ( @@ -531,6 +519,10 @@ def validate(vec_cls, value, *args, **kwargs): value, vec_cls.type.dim_name, is_array=True ) value = _unit_array_validator(value, vec_cls.type.dim) + + if kwargs.get("allow_inf_nan", False) is False: + value = _nan_inf_vector_validator(value) + value = _has_dimensions_validator(value, vec_cls.type.dim) return value diff --git a/tests/simulation/framework/test_unit_system_v2.py b/tests/simulation/framework/test_unit_system_v2.py index 852bd54fc..73ea21c35 100644 --- a/tests/simulation/framework/test_unit_system_v2.py +++ b/tests/simulation/framework/test_unit_system_v2.py @@ -4,6 +4,7 @@ import pydantic as pd import pytest +from numpy import nan from flow360.component.simulation import units as u from flow360.component.simulation.framework.base_model import Flow360BaseModel @@ -493,7 +494,7 @@ def test_unit_system(): with pytest.raises( pd.ValidationError, - match=r"Value error, NaN or None found in input array which is not allowed.", + match=r"NaN/Inf/None found in input array. Please ensure your input is complete.", ): data = VectorDataWithUnits( pt=None, From 2e49081fef30e46fa0033a49ec9e56132402bc70 Mon Sep 17 00:00:00 2001 From: BenYuan Date: Fri, 8 Nov 2024 19:21:29 +0000 Subject: [PATCH 3/4] Fixed unit test --- flow360/component/simulation/unit_system.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/flow360/component/simulation/unit_system.py b/flow360/component/simulation/unit_system.py index 18b4bb31a..18700daf1 100644 --- a/flow360/component/simulation/unit_system.py +++ b/flow360/component/simulation/unit_system.py @@ -255,6 +255,8 @@ def _has_dimensions_validator(value, dim): def _nan_inf_vector_validator(value): + if not isinstance(value, np.ndarray): + return value if np.ndim(value.value) > 0 and (any(np.isnan(value.value)) or any(np.isinf(value.value))): raise ValueError("NaN/Inf/None found in input array. Please ensure your input is complete.") return value From 0b4b1f3f27b20d4bf3d159594bf371307b2180c9 Mon Sep 17 00:00:00 2001 From: Ben <106089368+benflexcompute@users.noreply.github.com> Date: Wed, 13 Nov 2024 14:10:59 -0500 Subject: [PATCH 4/4] Update flow360/component/simulation/unit_system.py Co-authored-by: Maciej Skarysz <83596707+maciej-flexcompute@users.noreply.github.com> --- flow360/component/simulation/unit_system.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flow360/component/simulation/unit_system.py b/flow360/component/simulation/unit_system.py index e4d892fff..ed2282b34 100644 --- a/flow360/component/simulation/unit_system.py +++ b/flow360/component/simulation/unit_system.py @@ -378,7 +378,7 @@ class _ConType(pd.BaseModel): def validate(con_cls, value, *args, **kwargs): """Additional validator for value""" try: - dimensioned_value = dim_type.validate(value, kwargs) + dimensioned_value = dim_type.validate(value, **kwargs) # Workaround to run annotated validation for numeric value of field _ = _ConType(value=dimensioned_value.value)