From b847c7dd553b19ee7554644234c57816dd3d9d79 Mon Sep 17 00:00:00 2001 From: djalky Date: Mon, 10 Mar 2025 11:15:29 -0400 Subject: [PATCH 1/5] fixing non-determinism issue with parmest related to issue #3372 --- pyomo/contrib/parmest/parmest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyomo/contrib/parmest/parmest.py b/pyomo/contrib/parmest/parmest.py index 41e7792570b..01f2d3b0c91 100644 --- a/pyomo/contrib/parmest/parmest.py +++ b/pyomo/contrib/parmest/parmest.py @@ -309,7 +309,7 @@ def __init__( for experiment in self.exp_list: model = experiment.get_labeled_model() theta_names.extend([k.name for k, v in model.unknown_parameters.items()]) - self.estimator_theta_names = list(set(theta_names)) + self.estimator_theta_names = list(dict.fromkeys(theta_names)) self._second_stage_cost_exp = "SecondStageCost" # boolean to indicate if model is initialized using a square solve From f9755aab44932df5714239065fa690bdb616d116 Mon Sep 17 00:00:00 2001 From: djalky Date: Mon, 10 Mar 2025 11:21:09 -0400 Subject: [PATCH 2/5] fixing two bugs in doe contribution First bug does not revert the parameter perturbation during sequential FIM computation. Second bug was not properly populating the results object. --- pyomo/contrib/doe/doe.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pyomo/contrib/doe/doe.py b/pyomo/contrib/doe/doe.py index 3a616b714b8..643a7d651b5 100644 --- a/pyomo/contrib/doe/doe.py +++ b/pyomo/contrib/doe/doe.py @@ -551,6 +551,9 @@ def _sequential_FIM(self, model=None): "Model from experiment did not solve appropriately. Make sure the model is well-posed." ) + # Reset value of parameter to default value before computing finite difference perturbation + param.set_value(model.unknown_parameters[param]) + # Extract the measurement values for the scenario and append measurement_vals.append( [pyo.value(k) for k, v in model.experiment_outputs.items()] @@ -2140,7 +2143,7 @@ def get_unknown_parameter_values(self, model=None): if not hasattr(model, "unknown_parameters"): if not hasattr(model, "scenario_blocks"): raise RuntimeError( - "Model provided does not have expected structure. Please make sure model is built properly before calling `get_experiment_input_values`" + "Model provided does not have expected structure. Please make sure model is built properly before calling `get_unknown_parameter_values`" ) theta_vals = [ @@ -2174,15 +2177,15 @@ def get_experiment_output_values(self, model=None): if not hasattr(model, "experiment_outputs"): if not hasattr(model, "scenario_blocks"): raise RuntimeError( - "Model provided does not have expected structure. Please make sure model is built properly before calling `get_experiment_input_values`" + "Model provided does not have expected structure. Please make sure model is built properly before calling `get_experiment_output_values`" ) y_hat_vals = [ pyo.value(k) - for k, v in model.scenario_blocks[0].measurement_error.items() + for k, v in model.scenario_blocks[0].experiment_outputs.items() ] else: - y_hat_vals = [pyo.value(k) for k, v in model.measurement_error.items()] + y_hat_vals = [pyo.value(k) for k, v in model.experiment_outputs.items()] return y_hat_vals @@ -2210,7 +2213,7 @@ def get_measurement_error_values(self, model=None): if not hasattr(model, "measurement_error"): if not hasattr(model, "scenario_blocks"): raise RuntimeError( - "Model provided does not have expected structure. Please make sure model is built properly before calling `get_experiment_input_values`" + "Model provided does not have expected structure. Please make sure model is built properly before calling `get_measurement_error_values`" ) sigma_vals = [ From 2e56b6bd18d8059c10cccef5c3164dd2d443f393 Mon Sep 17 00:00:00 2001 From: djalky Date: Mon, 10 Mar 2025 11:33:11 -0400 Subject: [PATCH 3/5] Fix testing files in doe to reflect error updates Updated tests to match the corrected error messages. --- pyomo/contrib/doe/tests/test_doe_errors.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyomo/contrib/doe/tests/test_doe_errors.py b/pyomo/contrib/doe/tests/test_doe_errors.py index 52f6ef8cdb8..32098cdd829 100644 --- a/pyomo/contrib/doe/tests/test_doe_errors.py +++ b/pyomo/contrib/doe/tests/test_doe_errors.py @@ -498,7 +498,7 @@ def test_reactor_check_get_exp_outputs_without_model(self): with self.assertRaisesRegex( RuntimeError, - "Model provided does not have expected structure. Please make sure model is built properly before calling `get_experiment_input_values`", + "Model provided does not have expected structure. Please make sure model is built properly before calling `get_experiment_output_values`", ): doe_obj.get_experiment_output_values() @@ -517,7 +517,7 @@ def test_reactor_check_get_unknown_params_without_model(self): with self.assertRaisesRegex( RuntimeError, - "Model provided does not have expected structure. Please make sure model is built properly before calling `get_experiment_input_values`", + "Model provided does not have expected structure. Please make sure model is built properly before calling `get_unknown_parameter_values`", ): doe_obj.get_unknown_parameter_values() @@ -536,7 +536,7 @@ def test_reactor_check_get_meas_error_without_model(self): with self.assertRaisesRegex( RuntimeError, - "Model provided does not have expected structure. Please make sure model is built properly before calling `get_experiment_input_values`", + "Model provided does not have expected structure. Please make sure model is built properly before calling `get_measurement_error_values`", ): doe_obj.get_measurement_error_values() From aaeda4e8297c6db3c2e05f32ccc638a607572745 Mon Sep 17 00:00:00 2001 From: Daniel Laky <29078718+djlaky@users.noreply.github.com> Date: Wed, 12 Mar 2025 07:13:50 -0400 Subject: [PATCH 4/5] add comment for change in parmest Added comment to be explicit that dict.fromkeys() is used instead of set() to preserve parameter ordering and thus make the problem deterministic. --- pyomo/contrib/parmest/parmest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pyomo/contrib/parmest/parmest.py b/pyomo/contrib/parmest/parmest.py index d49659589fc..bf7294f2bc7 100644 --- a/pyomo/contrib/parmest/parmest.py +++ b/pyomo/contrib/parmest/parmest.py @@ -309,6 +309,7 @@ def __init__( for experiment in self.exp_list: model = experiment.get_labeled_model() theta_names.extend([k.name for k, v in model.unknown_parameters.items()]) + # Utilize list(dict.fromkeys(theta_names)) to preserve parameter order compared with list(set(theta_names)), which had nondeterministic ordering of parameters self.estimator_theta_names = list(dict.fromkeys(theta_names)) self._second_stage_cost_exp = "SecondStageCost" From 679f269e9eab495909425c113ade3573f3b7aae0 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 12 Mar 2025 06:29:49 -0600 Subject: [PATCH 5/5] Wrap comment to <88 columns (black) --- pyomo/contrib/parmest/parmest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pyomo/contrib/parmest/parmest.py b/pyomo/contrib/parmest/parmest.py index bf7294f2bc7..ea9dfc00640 100644 --- a/pyomo/contrib/parmest/parmest.py +++ b/pyomo/contrib/parmest/parmest.py @@ -309,7 +309,9 @@ def __init__( for experiment in self.exp_list: model = experiment.get_labeled_model() theta_names.extend([k.name for k, v in model.unknown_parameters.items()]) - # Utilize list(dict.fromkeys(theta_names)) to preserve parameter order compared with list(set(theta_names)), which had nondeterministic ordering of parameters + # Utilize list(dict.fromkeys(theta_names)) to preserve parameter + # order compared with list(set(theta_names)), which had + # nondeterministic ordering of parameters self.estimator_theta_names = list(dict.fromkeys(theta_names)) self._second_stage_cost_exp = "SecondStageCost"