From 06b8b1064e3a5f30d5b65d9f19dee00888b1f465 Mon Sep 17 00:00:00 2001 From: Gilberto RUIZ JIMENEZ Date: Wed, 6 Dec 2023 14:29:00 +0100 Subject: [PATCH 1/3] fix(MDODiscipline): check the discipline inputs before trying to retrieve them from the cache in MDODiscipline.execute() --- src/gemseo/core/discipline.py | 8 ++++---- tests/core/test_discipline.py | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/gemseo/core/discipline.py b/src/gemseo/core/discipline.py index 8b62c516fc..c709dcb925 100644 --- a/src/gemseo/core/discipline.py +++ b/src/gemseo/core/discipline.py @@ -970,11 +970,11 @@ class MDODiscipline(Serializable): * Adds the default inputs to the ``input_data`` if some inputs are not defined in input_data but exist in :attr:`.MDODiscipline.default_inputs`. + * Checks the input data against :attr:`.MDODiscipline.input_grammar`. * Checks whether the last execution of the discipline was called with identical inputs, i.e. cached in :attr:`.MDODiscipline.cache`; if so, directly returns ``self.cache.get_output_cache(inputs)``. * Caches the inputs. - * Checks the input data against :attr:`.MDODiscipline.input_grammar`. * If :attr:`.MDODiscipline.data_processor` is not None, runs the preprocessor. * Updates the status to :attr:`.MDODiscipline.ExecutionStatus.RUNNING`. * Calls the :meth:`.MDODiscipline._run` method, that shall be defined. @@ -996,6 +996,9 @@ class MDODiscipline(Serializable): # Load the default_inputs if the user did not provide all required data input_data = self._filter_inputs(input_data) + if self.activate_input_data_check: + self.check_input_data(input_data) + cached_local_data = self.__get_cache_data(input_data) if cached_local_data is not None: return cached_local_data @@ -1003,9 +1006,6 @@ class MDODiscipline(Serializable): # Cache was not loaded, see self.linearize self._cache_was_loaded = False - if self.activate_input_data_check: - self.check_input_data(input_data) - # Keep a pristine copy of the inputs before it is eventually changed. inputs_for_cache = self.__create_input_data_for_cache(input_data) diff --git a/tests/core/test_discipline.py b/tests/core/test_discipline.py index eb775de5d4..4060e2cd56 100644 --- a/tests/core/test_discipline.py +++ b/tests/core/test_discipline.py @@ -1320,3 +1320,22 @@ def test_set_cache_policy_to_none(): assert isinstance(discipline.cache, SimpleCache) discipline.set_cache_policy(discipline.CacheType.NONE) assert discipline.cache is None + + +class SimpleDiscipline(MDODiscipline): + def __init__(self): + super().__init__(grammar_type="JSONGrammar") + self.input_grammar.update_from_names(["x"]) + self.output_grammar.update_from_names(["y"]) + + def _run(self): + (x,) = self.get_inputs_by_name(["x"]) + self.local_data["y"] = array([2.0 * x[0]]) + + +def test_check_grammar_before_cache(): + """Test that a discipline does not return a cached result for invalid data.""" + discipline = SimpleDiscipline() + discipline.execute({"x": array([1.0])}) + with pytest.raises(InvalidDataError): + discipline.execute({"x": 1.0}) -- GitLab From be90dc578b5ebabe7cf7926e57437e8ba96f2b34 Mon Sep 17 00:00:00 2001 From: Gilberto RUIZ JIMENEZ Date: Wed, 6 Dec 2023 14:33:22 +0100 Subject: [PATCH 2/3] docs(Changelog): update the changelog --- changelog/fragments/991.fixed.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelog/fragments/991.fixed.rst diff --git a/changelog/fragments/991.fixed.rst b/changelog/fragments/991.fixed.rst new file mode 100644 index 0000000000..0a6550ffe5 --- /dev/null +++ b/changelog/fragments/991.fixed.rst @@ -0,0 +1,2 @@ +An ``MDODiscipline`` now checks the validity of the input data against the grammar before trying to retrieve them +from the cache. -- GitLab From ade2be362ad442941e03540ed4449367c8bd0555 Mon Sep 17 00:00:00 2001 From: Gilberto RUIZ JIMENEZ Date: Fri, 8 Dec 2023 17:28:29 +0100 Subject: [PATCH 3/3] refactor(Comparisons): add a condition to check the value type instead of doing it with the grammar so that different types are not considered equal --- src/gemseo/core/discipline.py | 8 ++++---- src/gemseo/utils/comparisons.py | 7 +++++++ tests/core/test_discipline.py | 9 +++++++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/gemseo/core/discipline.py b/src/gemseo/core/discipline.py index c709dcb925..8b62c516fc 100644 --- a/src/gemseo/core/discipline.py +++ b/src/gemseo/core/discipline.py @@ -970,11 +970,11 @@ class MDODiscipline(Serializable): * Adds the default inputs to the ``input_data`` if some inputs are not defined in input_data but exist in :attr:`.MDODiscipline.default_inputs`. - * Checks the input data against :attr:`.MDODiscipline.input_grammar`. * Checks whether the last execution of the discipline was called with identical inputs, i.e. cached in :attr:`.MDODiscipline.cache`; if so, directly returns ``self.cache.get_output_cache(inputs)``. * Caches the inputs. + * Checks the input data against :attr:`.MDODiscipline.input_grammar`. * If :attr:`.MDODiscipline.data_processor` is not None, runs the preprocessor. * Updates the status to :attr:`.MDODiscipline.ExecutionStatus.RUNNING`. * Calls the :meth:`.MDODiscipline._run` method, that shall be defined. @@ -996,9 +996,6 @@ class MDODiscipline(Serializable): # Load the default_inputs if the user did not provide all required data input_data = self._filter_inputs(input_data) - if self.activate_input_data_check: - self.check_input_data(input_data) - cached_local_data = self.__get_cache_data(input_data) if cached_local_data is not None: return cached_local_data @@ -1006,6 +1003,9 @@ class MDODiscipline(Serializable): # Cache was not loaded, see self.linearize self._cache_was_loaded = False + if self.activate_input_data_check: + self.check_input_data(input_data) + # Keep a pristine copy of the inputs before it is eventually changed. inputs_for_cache = self.__create_input_data_for_cache(input_data) diff --git a/src/gemseo/utils/comparisons.py b/src/gemseo/utils/comparisons.py index 432fe63319..a0e2e7bddd 100644 --- a/src/gemseo/utils/comparisons.py +++ b/src/gemseo/utils/comparisons.py @@ -23,7 +23,9 @@ from collections.abc import Mapping from typing import Union from numpy import asarray +from numpy import ndarray from numpy.linalg import norm +from scipy.sparse import spmatrix from scipy.sparse.linalg import norm as spnorm from gemseo.utils.compatibility.scipy import ArrayType @@ -66,6 +68,9 @@ def compare_dict_of_arrays( # Check the values if tolerance: for key, value in dict_of_arrays.items(): + if not isinstance(value, (ndarray, spmatrix)): + return False + difference = other_dict_of_arrays[key] - value if isinstance(difference, sparse_classes): @@ -81,6 +86,8 @@ def compare_dict_of_arrays( return False else: for key, value in dict_of_arrays.items(): + if not isinstance(value, (ndarray, spmatrix)): + return False is_different = other_dict_of_arrays[key] != value if isinstance(is_different, sparse_classes): diff --git a/tests/core/test_discipline.py b/tests/core/test_discipline.py index 4060e2cd56..ea2dbd1834 100644 --- a/tests/core/test_discipline.py +++ b/tests/core/test_discipline.py @@ -1333,8 +1333,13 @@ class SimpleDiscipline(MDODiscipline): self.local_data["y"] = array([2.0 * x[0]]) -def test_check_grammar_before_cache(): - """Test that a discipline does not return a cached result for invalid data.""" +def test_grammar_same_input_different_types(): + """Test that a discipline does not return a cached result for invalid data. + + The hash of cached 1d array entry is the same as the hash of the same data as a + float, here we test that even if an entry already exists for a numpy array, the + cache will not return the cached result if the types are different. + """ discipline = SimpleDiscipline() discipline.execute({"x": array([1.0])}) with pytest.raises(InvalidDataError): -- GitLab