From c571f5c2d1952db77ea980d472a90d64463a355f Mon Sep 17 00:00:00 2001 From: robbybp Date: Tue, 12 Dec 2023 08:19:04 -0700 Subject: [PATCH 01/20] initial implementation of identify-via-amplrepn --- pyomo/contrib/incidence_analysis/incidence.py | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/pyomo/contrib/incidence_analysis/incidence.py b/pyomo/contrib/incidence_analysis/incidence.py index 1852cf75648..974153984d2 100644 --- a/pyomo/contrib/incidence_analysis/incidence.py +++ b/pyomo/contrib/incidence_analysis/incidence.py @@ -16,6 +16,8 @@ from pyomo.core.expr.visitor import identify_variables from pyomo.core.expr.numvalue import value as pyo_value from pyomo.repn import generate_standard_repn +from pyomo.repn.nl_writer import AMPLRepnVisitor, AMPLRepn, text_nl_template +from pyomo.repn.util import FileDeterminism, FileDeterminism_to_SortComponents from pyomo.util.subsystems import TemporarySubsystemManager from pyomo.contrib.incidence_analysis.config import IncidenceMethod, IncidenceConfig @@ -74,6 +76,45 @@ def _get_incident_via_standard_repn(expr, include_fixed, linear_only): return unique_variables +def _get_incident_via_amplrepn(expr, linear_only): + subexpression_cache = {} + subexpression_order = [] + external_functions = {} + var_map = {} + used_named_expressions = set() + symbolic_solver_labels = False + export_defined_variabels = True + sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) + visitor = AMPLRepnVisitor( + text_nl_template, + subexpression_cache, + subexpression_order, + external_functions, + var_map, + used_named_expressions, + symbolic_solver_labels, + export_defined_variables, + sorter, + ) + AMPLRepn.ActiveVisitor = visitor + try: + repn = visitor.walk_expression((expr, None, 0, 1.0)) + finally: + AMPLRepn.ActiveVisitor = None + + nonlinear_vars = [var_map[v_id] for v_id in repn.nonlinear[1]] + nonlinear_vid_set = set(repn.nonlinear[1]) + linear_only_vars = [ + var_map[v_id] for v_id, coef in repn.linear.items() + if coef != 0.0 and v_id not in nonlinear_vid_set + ] + if linear_only: + return linear_only_vars + else: + variables = linear_only_vars + nonlinear_vars + return variables + + def get_incident_variables(expr, **kwds): """Get variables that participate in an expression From 07c65e940a32a5fae3c6509c4a055c28ef4b146b Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Tue, 12 Dec 2023 09:53:14 -0700 Subject: [PATCH 02/20] add IncidenceMethod.ampl_repn option --- pyomo/contrib/incidence_analysis/config.py | 3 +++ pyomo/contrib/incidence_analysis/incidence.py | 17 ++++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/config.py b/pyomo/contrib/incidence_analysis/config.py index 56841617cac..60acc53abfc 100644 --- a/pyomo/contrib/incidence_analysis/config.py +++ b/pyomo/contrib/incidence_analysis/config.py @@ -24,6 +24,9 @@ class IncidenceMethod(enum.Enum): standard_repn = 1 """Use ``pyomo.repn.standard_repn.generate_standard_repn``""" + ampl_repn = 2 + """Use ``pyomo.repn.plugins.nl_writer.AMPLRepnVisitor``""" + _include_fixed = ConfigValue( default=False, diff --git a/pyomo/contrib/incidence_analysis/incidence.py b/pyomo/contrib/incidence_analysis/incidence.py index 974153984d2..f16b248463c 100644 --- a/pyomo/contrib/incidence_analysis/incidence.py +++ b/pyomo/contrib/incidence_analysis/incidence.py @@ -16,7 +16,7 @@ from pyomo.core.expr.visitor import identify_variables from pyomo.core.expr.numvalue import value as pyo_value from pyomo.repn import generate_standard_repn -from pyomo.repn.nl_writer import AMPLRepnVisitor, AMPLRepn, text_nl_template +from pyomo.repn.plugins.nl_writer import AMPLRepnVisitor, AMPLRepn, text_nl_template from pyomo.repn.util import FileDeterminism, FileDeterminism_to_SortComponents from pyomo.util.subsystems import TemporarySubsystemManager from pyomo.contrib.incidence_analysis.config import IncidenceMethod, IncidenceConfig @@ -76,14 +76,14 @@ def _get_incident_via_standard_repn(expr, include_fixed, linear_only): return unique_variables -def _get_incident_via_amplrepn(expr, linear_only): +def _get_incident_via_ampl_repn(expr, linear_only): subexpression_cache = {} subexpression_order = [] external_functions = {} var_map = {} used_named_expressions = set() symbolic_solver_labels = False - export_defined_variabels = True + export_defined_variables = True sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) visitor = AMPLRepnVisitor( text_nl_template, @@ -102,8 +102,9 @@ def _get_incident_via_amplrepn(expr, linear_only): finally: AMPLRepn.ActiveVisitor = None - nonlinear_vars = [var_map[v_id] for v_id in repn.nonlinear[1]] - nonlinear_vid_set = set(repn.nonlinear[1]) + nonlinear_var_ids = [] if repn.nonlinear is None else repn.nonlinear[1] + nonlinear_vars = [var_map[v_id] for v_id in nonlinear_var_ids] + nonlinear_vid_set = set(nonlinear_var_ids) linear_only_vars = [ var_map[v_id] for v_id, coef in repn.linear.items() if coef != 0.0 and v_id not in nonlinear_vid_set @@ -161,10 +162,16 @@ def get_incident_variables(expr, **kwds): raise RuntimeError( "linear_only=True is not supported when using identify_variables" ) + if include_fixed and method is IncidenceMethod.ampl_repn: + raise RuntimeError( + "include_fixed=True is not supported when using ampl_repn" + ) if method is IncidenceMethod.identify_variables: return _get_incident_via_identify_variables(expr, include_fixed) elif method is IncidenceMethod.standard_repn: return _get_incident_via_standard_repn(expr, include_fixed, linear_only) + elif method is IncidenceMethod.ampl_repn: + return _get_incident_via_ampl_repn(expr, linear_only) else: raise ValueError( f"Unrecognized value {method} for the method used to identify incident" From 5c88a9794ad2d05595eb849d53af3d18a50747ab Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Tue, 12 Dec 2023 09:53:57 -0700 Subject: [PATCH 03/20] refactor tests and test ampl_repn option --- .../tests/test_incidence.py | 124 ++++++++++++------ 1 file changed, 83 insertions(+), 41 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/tests/test_incidence.py b/pyomo/contrib/incidence_analysis/tests/test_incidence.py index 7f57dd904a7..e37e4f97691 100644 --- a/pyomo/contrib/incidence_analysis/tests/test_incidence.py +++ b/pyomo/contrib/incidence_analysis/tests/test_incidence.py @@ -63,37 +63,37 @@ def test_incidence_with_fixed_variable(self): var_set = ComponentSet(variables) self.assertEqual(var_set, ComponentSet([m.x[1], m.x[3]])) - def test_incidence_with_mutable_parameter(self): + +class _TestIncidenceLinearOnly(object): + def _get_incident_variables(self, expr): + raise NotImplementedError( + "_TestIncidenceLinearOnly should not be used directly" + ) + + def test_linear_only(self): m = pyo.ConcreteModel() m.x = pyo.Var([1, 2, 3]) - m.p = pyo.Param(mutable=True, initialize=None) - expr = m.x[1] + m.p * m.x[1] * m.x[2] + m.x[1] * pyo.exp(m.x[3]) - variables = self._get_incident_variables(expr) - self.assertEqual(ComponentSet(variables), ComponentSet(m.x[:])) + expr = 2 * m.x[1] + 4 * m.x[2] * m.x[1] - m.x[1] * pyo.exp(m.x[3]) + variables = self._get_incident_variables(expr, linear_only=True) + self.assertEqual(len(variables), 0) -class TestIncidenceStandardRepn(unittest.TestCase, _TestIncidence): - def _get_incident_variables(self, expr, **kwds): - method = IncidenceMethod.standard_repn - return get_incident_variables(expr, method=method, **kwds) + expr = 2 * m.x[1] + 2 * m.x[2] * m.x[3] + 3 * m.x[2] + variables = self._get_incident_variables(expr, linear_only=True) + self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1]])) - def test_assumed_standard_repn_behavior(self): - m = pyo.ConcreteModel() - m.x = pyo.Var([1, 2]) - m.p = pyo.Param(initialize=0.0) + m.x[3].fix(2.5) + expr = 2 * m.x[1] + 2 * m.x[2] * m.x[3] + 3 * m.x[2] + variables = self._get_incident_variables(expr, linear_only=True) + self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1], m.x[2]])) - # We rely on variables with constant coefficients of zero not appearing - # in the standard repn (as opposed to appearing with explicit - # coefficients of zero). - expr = m.x[1] + 0 * m.x[2] - repn = generate_standard_repn(expr) - self.assertEqual(len(repn.linear_vars), 1) - self.assertIs(repn.linear_vars[0], m.x[1]) - expr = m.p * m.x[1] + m.x[2] - repn = generate_standard_repn(expr) - self.assertEqual(len(repn.linear_vars), 1) - self.assertIs(repn.linear_vars[0], m.x[2]) +class _TestIncidenceLinearCancellation(object): + """Tests for methods that perform linear cancellation""" + def _get_incident_variables(self, expr): + raise NotImplementedError( + "_TestIncidenceLinearCancellation should not be used directly" + ) def test_zero_coef(self): m = pyo.ConcreteModel() @@ -113,23 +113,6 @@ def test_variable_minus_itself(self): var_set = ComponentSet(variables) self.assertEqual(var_set, ComponentSet([m.x[2], m.x[3]])) - def test_linear_only(self): - m = pyo.ConcreteModel() - m.x = pyo.Var([1, 2, 3]) - - expr = 2 * m.x[1] + 4 * m.x[2] * m.x[1] - m.x[1] * pyo.exp(m.x[3]) - variables = self._get_incident_variables(expr, linear_only=True) - self.assertEqual(len(variables), 0) - - expr = 2 * m.x[1] + 2 * m.x[2] * m.x[3] + 3 * m.x[2] - variables = self._get_incident_variables(expr, linear_only=True) - self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1]])) - - m.x[3].fix(2.5) - expr = 2 * m.x[1] + 2 * m.x[2] * m.x[3] + 3 * m.x[2] - variables = self._get_incident_variables(expr, linear_only=True) - self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1], m.x[2]])) - def test_fixed_zero_linear_coefficient(self): m = pyo.ConcreteModel() m.x = pyo.Var([1, 2, 3]) @@ -148,6 +131,9 @@ def test_fixed_zero_linear_coefficient(self): variables = self._get_incident_variables(expr) self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1], m.x[2]])) + # NOTE: This test assumes that all methods that support linear cancellation + # accept a linear_only argument. If this changes, this test wil need to be + # moved. def test_fixed_zero_coefficient_linear_only(self): m = pyo.ConcreteModel() m.x = pyo.Var([1, 2, 3]) @@ -159,6 +145,35 @@ def test_fixed_zero_coefficient_linear_only(self): self.assertEqual(len(variables), 1) self.assertIs(variables[0], m.x[3]) + +class TestIncidenceStandardRepn( + unittest.TestCase, + _TestIncidence, + _TestIncidenceLinearOnly, + _TestIncidenceLinearCancellation, +): + def _get_incident_variables(self, expr, **kwds): + method = IncidenceMethod.standard_repn + return get_incident_variables(expr, method=method, **kwds) + + def test_assumed_standard_repn_behavior(self): + m = pyo.ConcreteModel() + m.x = pyo.Var([1, 2]) + m.p = pyo.Param(initialize=0.0) + + # We rely on variables with constant coefficients of zero not appearing + # in the standard repn (as opposed to appearing with explicit + # coefficients of zero). + expr = m.x[1] + 0 * m.x[2] + repn = generate_standard_repn(expr) + self.assertEqual(len(repn.linear_vars), 1) + self.assertIs(repn.linear_vars[0], m.x[1]) + + expr = m.p * m.x[1] + m.x[2] + repn = generate_standard_repn(expr) + self.assertEqual(len(repn.linear_vars), 1) + self.assertIs(repn.linear_vars[0], m.x[2]) + def test_fixed_none_linear_coefficient(self): m = pyo.ConcreteModel() m.x = pyo.Var([1, 2, 3]) @@ -168,6 +183,14 @@ def test_fixed_none_linear_coefficient(self): variables = self._get_incident_variables(expr) self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1], m.x[2]])) + def test_incidence_with_mutable_parameter(self): + m = pyo.ConcreteModel() + m.x = pyo.Var([1, 2, 3]) + m.p = pyo.Param(mutable=True, initialize=None) + expr = m.x[1] + m.p * m.x[1] * m.x[2] + m.x[1] * pyo.exp(m.x[3]) + variables = self._get_incident_variables(expr) + self.assertEqual(ComponentSet(variables), ComponentSet(m.x[:])) + class TestIncidenceIdentifyVariables(unittest.TestCase, _TestIncidence): def _get_incident_variables(self, expr, **kwds): @@ -192,6 +215,25 @@ def test_variable_minus_itself(self): var_set = ComponentSet(variables) self.assertEqual(var_set, ComponentSet(m.x[:])) + def test_incidence_with_mutable_parameter(self): + m = pyo.ConcreteModel() + m.x = pyo.Var([1, 2, 3]) + m.p = pyo.Param(mutable=True, initialize=None) + expr = m.x[1] + m.p * m.x[1] * m.x[2] + m.x[1] * pyo.exp(m.x[3]) + variables = self._get_incident_variables(expr) + self.assertEqual(ComponentSet(variables), ComponentSet(m.x[:])) + + +class TestIncidenceAmplRepn( + unittest.TestCase, + _TestIncidence, + _TestIncidenceLinearOnly, + _TestIncidenceLinearCancellation, +): + def _get_incident_variables(self, expr, **kwds): + method = IncidenceMethod.ampl_repn + return get_incident_variables(expr, method=method, **kwds) + if __name__ == "__main__": unittest.main() From 515f7e59705ec6b95f6784f42a69ed2b1517161f Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Tue, 12 Dec 2023 12:09:44 -0700 Subject: [PATCH 04/20] apply black --- pyomo/contrib/incidence_analysis/incidence.py | 9 ++++----- pyomo/contrib/incidence_analysis/tests/test_incidence.py | 1 + 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/incidence.py b/pyomo/contrib/incidence_analysis/incidence.py index f16b248463c..5ac7b49fa1f 100644 --- a/pyomo/contrib/incidence_analysis/incidence.py +++ b/pyomo/contrib/incidence_analysis/incidence.py @@ -103,10 +103,11 @@ def _get_incident_via_ampl_repn(expr, linear_only): AMPLRepn.ActiveVisitor = None nonlinear_var_ids = [] if repn.nonlinear is None else repn.nonlinear[1] - nonlinear_vars = [var_map[v_id] for v_id in nonlinear_var_ids] + nonlinear_vars = [var_map[v_id] for v_id in nonlinear_var_ids] nonlinear_vid_set = set(nonlinear_var_ids) linear_only_vars = [ - var_map[v_id] for v_id, coef in repn.linear.items() + var_map[v_id] + for v_id, coef in repn.linear.items() if coef != 0.0 and v_id not in nonlinear_vid_set ] if linear_only: @@ -163,9 +164,7 @@ def get_incident_variables(expr, **kwds): "linear_only=True is not supported when using identify_variables" ) if include_fixed and method is IncidenceMethod.ampl_repn: - raise RuntimeError( - "include_fixed=True is not supported when using ampl_repn" - ) + raise RuntimeError("include_fixed=True is not supported when using ampl_repn") if method is IncidenceMethod.identify_variables: return _get_incident_via_identify_variables(expr, include_fixed) elif method is IncidenceMethod.standard_repn: diff --git a/pyomo/contrib/incidence_analysis/tests/test_incidence.py b/pyomo/contrib/incidence_analysis/tests/test_incidence.py index e37e4f97691..bcf867c619a 100644 --- a/pyomo/contrib/incidence_analysis/tests/test_incidence.py +++ b/pyomo/contrib/incidence_analysis/tests/test_incidence.py @@ -90,6 +90,7 @@ def test_linear_only(self): class _TestIncidenceLinearCancellation(object): """Tests for methods that perform linear cancellation""" + def _get_incident_variables(self, expr): raise NotImplementedError( "_TestIncidenceLinearCancellation should not be used directly" From 8d5c737f551b3a513e13499957cc20dba86ec771 Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Tue, 12 Dec 2023 12:10:58 -0700 Subject: [PATCH 05/20] add docstring to TestLinearOnly helper class --- pyomo/contrib/incidence_analysis/tests/test_incidence.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyomo/contrib/incidence_analysis/tests/test_incidence.py b/pyomo/contrib/incidence_analysis/tests/test_incidence.py index bcf867c619a..78493ecc651 100644 --- a/pyomo/contrib/incidence_analysis/tests/test_incidence.py +++ b/pyomo/contrib/incidence_analysis/tests/test_incidence.py @@ -65,6 +65,8 @@ def test_incidence_with_fixed_variable(self): class _TestIncidenceLinearOnly(object): + """Tests for methods that support linear_only""" + def _get_incident_variables(self, expr): raise NotImplementedError( "_TestIncidenceLinearOnly should not be used directly" From 45eb8616c67058b895ce49ebca9aa61877731976 Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Tue, 12 Dec 2023 12:12:35 -0700 Subject: [PATCH 06/20] fix typo --- pyomo/contrib/incidence_analysis/tests/test_incidence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyomo/contrib/incidence_analysis/tests/test_incidence.py b/pyomo/contrib/incidence_analysis/tests/test_incidence.py index 78493ecc651..87a9178dc1a 100644 --- a/pyomo/contrib/incidence_analysis/tests/test_incidence.py +++ b/pyomo/contrib/incidence_analysis/tests/test_incidence.py @@ -135,7 +135,7 @@ def test_fixed_zero_linear_coefficient(self): self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1], m.x[2]])) # NOTE: This test assumes that all methods that support linear cancellation - # accept a linear_only argument. If this changes, this test wil need to be + # accept a linear_only argument. If this changes, this test will need to be # moved. def test_fixed_zero_coefficient_linear_only(self): m = pyo.ConcreteModel() From 682e054a217cbc37a8c444efd38d2df491603cd3 Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Tue, 12 Dec 2023 13:48:44 -0700 Subject: [PATCH 07/20] set export_defined_variables=False and add TODO comment about exploiting this later --- pyomo/contrib/incidence_analysis/incidence.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pyomo/contrib/incidence_analysis/incidence.py b/pyomo/contrib/incidence_analysis/incidence.py index 5ac7b49fa1f..b2cb23dc8c7 100644 --- a/pyomo/contrib/incidence_analysis/incidence.py +++ b/pyomo/contrib/incidence_analysis/incidence.py @@ -83,7 +83,10 @@ def _get_incident_via_ampl_repn(expr, linear_only): var_map = {} used_named_expressions = set() symbolic_solver_labels = False - export_defined_variables = True + # TODO: Explore potential performance benefit of exporting defined variables. + # This likely only shows up if we can preserve the subexpression cache across + # multiple constraint expressions. + export_defined_variables = False sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) visitor = AMPLRepnVisitor( text_nl_template, From c8ed1cda1a562788348157e72f90e103421af5d7 Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Tue, 12 Dec 2023 13:53:09 -0700 Subject: [PATCH 08/20] add test that uses named expression --- pyomo/contrib/incidence_analysis/tests/test_incidence.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pyomo/contrib/incidence_analysis/tests/test_incidence.py b/pyomo/contrib/incidence_analysis/tests/test_incidence.py index 87a9178dc1a..2354b0efc39 100644 --- a/pyomo/contrib/incidence_analysis/tests/test_incidence.py +++ b/pyomo/contrib/incidence_analysis/tests/test_incidence.py @@ -63,6 +63,15 @@ def test_incidence_with_fixed_variable(self): var_set = ComponentSet(variables) self.assertEqual(var_set, ComponentSet([m.x[1], m.x[3]])) + def test_incidence_with_named_expression(self): + m = pyo.ConcreteModel() + m.x = pyo.Var([1, 2, 3]) + m.subexpr = pyo.Expression(pyo.Integers) + m.subexpr[1] = m.x[1] * pyo.exp(m.x[3]) + expr = m.x[1] + m.x[1] * m.x[2] + m.subexpr[1] + variables = self._get_incident_variables(expr) + self.assertEqual(ComponentSet(variables), ComponentSet(m.x[:])) + class _TestIncidenceLinearOnly(object): """Tests for methods that support linear_only""" From 6675566fe7f4c7bf19832a5a72c09e7235cb6c8e Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Wed, 13 Dec 2023 13:19:16 -0700 Subject: [PATCH 09/20] re-use visitor when iterating over constraints --- pyomo/contrib/incidence_analysis/incidence.py | 61 ++++++++++--------- pyomo/contrib/incidence_analysis/interface.py | 39 ++++++++++-- 2 files changed, 67 insertions(+), 33 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/incidence.py b/pyomo/contrib/incidence_analysis/incidence.py index b2cb23dc8c7..7632f81e38a 100644 --- a/pyomo/contrib/incidence_analysis/incidence.py +++ b/pyomo/contrib/incidence_analysis/incidence.py @@ -76,34 +76,38 @@ def _get_incident_via_standard_repn(expr, include_fixed, linear_only): return unique_variables -def _get_incident_via_ampl_repn(expr, linear_only): - subexpression_cache = {} - subexpression_order = [] - external_functions = {} - var_map = {} - used_named_expressions = set() - symbolic_solver_labels = False - # TODO: Explore potential performance benefit of exporting defined variables. - # This likely only shows up if we can preserve the subexpression cache across - # multiple constraint expressions. - export_defined_variables = False - sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) - visitor = AMPLRepnVisitor( - text_nl_template, - subexpression_cache, - subexpression_order, - external_functions, - var_map, - used_named_expressions, - symbolic_solver_labels, - export_defined_variables, - sorter, - ) - AMPLRepn.ActiveVisitor = visitor - try: +def _get_incident_via_ampl_repn(expr, linear_only, visitor=None): + if visitor is None: + subexpression_cache = {} + subexpression_order = [] + external_functions = {} + var_map = {} + used_named_expressions = set() + symbolic_solver_labels = False + # TODO: Explore potential performance benefit of exporting defined variables. + # This likely only shows up if we can preserve the subexpression cache across + # multiple constraint expressions. + export_defined_variables = False + sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) + visitor = AMPLRepnVisitor( + text_nl_template, + subexpression_cache, + subexpression_order, + external_functions, + var_map, + used_named_expressions, + symbolic_solver_labels, + export_defined_variables, + sorter, + ) + AMPLRepn.ActiveVisitor = visitor + try: + repn = visitor.walk_expression((expr, None, 0, 1.0)) + finally: + AMPLRepn.ActiveVisitor = None + else: + var_map = visitor.var_map repn = visitor.walk_expression((expr, None, 0, 1.0)) - finally: - AMPLRepn.ActiveVisitor = None nonlinear_var_ids = [] if repn.nonlinear is None else repn.nonlinear[1] nonlinear_vars = [var_map[v_id] for v_id in nonlinear_var_ids] @@ -158,6 +162,7 @@ def get_incident_variables(expr, **kwds): ['x[1]', 'x[2]'] """ + visitor = kwds.pop("visitor", None) config = IncidenceConfig(kwds) method = config.method include_fixed = config.include_fixed @@ -173,7 +178,7 @@ def get_incident_variables(expr, **kwds): elif method is IncidenceMethod.standard_repn: return _get_incident_via_standard_repn(expr, include_fixed, linear_only) elif method is IncidenceMethod.ampl_repn: - return _get_incident_via_ampl_repn(expr, linear_only) + return _get_incident_via_ampl_repn(expr, linear_only, visitor=visitor) else: raise ValueError( f"Unrecognized value {method} for the method used to identify incident" diff --git a/pyomo/contrib/incidence_analysis/interface.py b/pyomo/contrib/incidence_analysis/interface.py index e922551c6a4..60e77d26f7a 100644 --- a/pyomo/contrib/incidence_analysis/interface.py +++ b/pyomo/contrib/incidence_analysis/interface.py @@ -29,7 +29,7 @@ plotly, ) from pyomo.common.deprecation import deprecated -from pyomo.contrib.incidence_analysis.config import IncidenceConfig +from pyomo.contrib.incidence_analysis.config import IncidenceConfig, IncidenceMethod from pyomo.contrib.incidence_analysis.matching import maximum_matching from pyomo.contrib.incidence_analysis.connected import get_independent_submatrices from pyomo.contrib.incidence_analysis.triangularize import ( @@ -45,6 +45,8 @@ ) from pyomo.contrib.incidence_analysis.incidence import get_incident_variables from pyomo.contrib.pynumero.asl import AmplInterface +from pyomo.repn.plugins.nl_writer import AMPLRepnVisitor, AMPLRepn, text_nl_template +from pyomo.repn.util import FileDeterminism, FileDeterminism_to_SortComponents pyomo_nlp, pyomo_nlp_available = attempt_import( 'pyomo.contrib.pynumero.interfaces.pyomo_nlp' @@ -99,10 +101,37 @@ def get_bipartite_incidence_graph(variables, constraints, **kwds): graph.add_nodes_from(range(M), bipartite=0) graph.add_nodes_from(range(M, M + N), bipartite=1) var_node_map = ComponentMap((v, M + i) for i, v in enumerate(variables)) - for i, con in enumerate(constraints): - for var in get_incident_variables(con.body, **config): - if var in var_node_map: - graph.add_edge(i, var_node_map[var]) + + if config.method == IncidenceMethod.ampl_repn: + subexpression_cache = {} + subexpression_order = [] + external_functions = {} + used_named_expressions = set() + symbolic_solver_labels = False + export_defined_variables = False + sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) + visitor = AMPLRepnVisitor( + text_nl_template, + subexpression_cache, + subexpression_order, + external_functions, + var_map, + used_named_expressions, + symbolic_solver_labels, + export_defined_variables, + sorter, + ) + else: + visitor = None + + AMPLRepn.ActiveVisitor = visitor + try: + for i, con in enumerate(constraints): + for var in get_incident_variables(con.body, visitor=visitor, **config): + if var in var_node_map: + graph.add_edge(i, var_node_map[var]) + finally: + AMPLRepn.ActiveVisitor = None return graph From bcd2435ebca336996893bfcb1243a54f3055d7f0 Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Wed, 13 Dec 2023 13:40:40 -0700 Subject: [PATCH 10/20] add IncidenceMethod.standard_repn_compute_values option --- pyomo/contrib/incidence_analysis/config.py | 5 + pyomo/contrib/incidence_analysis/incidence.py | 16 ++- .../tests/test_incidence.py | 132 ++++++++++++------ 3 files changed, 111 insertions(+), 42 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/config.py b/pyomo/contrib/incidence_analysis/config.py index a107792a9cd..62856047121 100644 --- a/pyomo/contrib/incidence_analysis/config.py +++ b/pyomo/contrib/incidence_analysis/config.py @@ -24,6 +24,11 @@ class IncidenceMethod(enum.Enum): standard_repn = 1 """Use ``pyomo.repn.standard_repn.generate_standard_repn``""" + standard_repn_compute_values = 2 + """Use ``pyomo.repn.standard_repn.generate_standard_repn`` with + ``compute_values=True`` + """ + _include_fixed = ConfigValue( default=False, diff --git a/pyomo/contrib/incidence_analysis/incidence.py b/pyomo/contrib/incidence_analysis/incidence.py index 1852cf75648..b8dcd27c685 100644 --- a/pyomo/contrib/incidence_analysis/incidence.py +++ b/pyomo/contrib/incidence_analysis/incidence.py @@ -29,7 +29,9 @@ def _get_incident_via_identify_variables(expr, include_fixed): return list(identify_variables(expr, include_fixed=include_fixed)) -def _get_incident_via_standard_repn(expr, include_fixed, linear_only): +def _get_incident_via_standard_repn( + expr, include_fixed, linear_only, compute_values=False +): if include_fixed: to_unfix = [ var for var in identify_variables(expr, include_fixed=True) if var.fixed @@ -39,7 +41,9 @@ def _get_incident_via_standard_repn(expr, include_fixed, linear_only): context = nullcontext() with context: - repn = generate_standard_repn(expr, compute_values=False, quadratic=False) + repn = generate_standard_repn( + expr, compute_values=compute_values, quadratic=False + ) linear_vars = [] # Check coefficients to make sure we don't include linear variables with @@ -123,7 +127,13 @@ def get_incident_variables(expr, **kwds): if method is IncidenceMethod.identify_variables: return _get_incident_via_identify_variables(expr, include_fixed) elif method is IncidenceMethod.standard_repn: - return _get_incident_via_standard_repn(expr, include_fixed, linear_only) + return _get_incident_via_standard_repn( + expr, include_fixed, linear_only, compute_values=False + ) + elif method is IncidenceMethod.standard_repn_compute_values: + return _get_incident_via_standard_repn( + expr, include_fixed, linear_only, compute_values=True + ) else: raise ValueError( f"Unrecognized value {method} for the method used to identify incident" diff --git a/pyomo/contrib/incidence_analysis/tests/test_incidence.py b/pyomo/contrib/incidence_analysis/tests/test_incidence.py index 7f57dd904a7..b1a8ef1b14c 100644 --- a/pyomo/contrib/incidence_analysis/tests/test_incidence.py +++ b/pyomo/contrib/incidence_analysis/tests/test_incidence.py @@ -56,44 +56,56 @@ def test_basic_incidence(self): def test_incidence_with_fixed_variable(self): m = pyo.ConcreteModel() - m.x = pyo.Var([1, 2, 3]) + m.x = pyo.Var([1, 2, 3], initialize=1.0) expr = m.x[1] + m.x[1] * m.x[2] + m.x[1] * pyo.exp(m.x[3]) m.x[2].fix() variables = self._get_incident_variables(expr) var_set = ComponentSet(variables) self.assertEqual(var_set, ComponentSet([m.x[1], m.x[3]])) - def test_incidence_with_mutable_parameter(self): + def test_incidence_with_named_expression(self): m = pyo.ConcreteModel() m.x = pyo.Var([1, 2, 3]) - m.p = pyo.Param(mutable=True, initialize=None) - expr = m.x[1] + m.p * m.x[1] * m.x[2] + m.x[1] * pyo.exp(m.x[3]) + m.subexpr = pyo.Expression(pyo.Integers) + m.subexpr[1] = m.x[1] * pyo.exp(m.x[3]) + expr = m.x[1] + m.x[1] * m.x[2] + m.subexpr[1] variables = self._get_incident_variables(expr) self.assertEqual(ComponentSet(variables), ComponentSet(m.x[:])) -class TestIncidenceStandardRepn(unittest.TestCase, _TestIncidence): - def _get_incident_variables(self, expr, **kwds): - method = IncidenceMethod.standard_repn - return get_incident_variables(expr, method=method, **kwds) +class _TestIncidenceLinearOnly(object): + """Tests for methods that support linear_only""" - def test_assumed_standard_repn_behavior(self): + def _get_incident_variables(self, expr): + raise NotImplementedError( + "_TestIncidenceLinearOnly should not be used directly" + ) + + def test_linear_only(self): m = pyo.ConcreteModel() - m.x = pyo.Var([1, 2]) - m.p = pyo.Param(initialize=0.0) + m.x = pyo.Var([1, 2, 3]) - # We rely on variables with constant coefficients of zero not appearing - # in the standard repn (as opposed to appearing with explicit - # coefficients of zero). - expr = m.x[1] + 0 * m.x[2] - repn = generate_standard_repn(expr) - self.assertEqual(len(repn.linear_vars), 1) - self.assertIs(repn.linear_vars[0], m.x[1]) + expr = 2 * m.x[1] + 4 * m.x[2] * m.x[1] - m.x[1] * pyo.exp(m.x[3]) + variables = self._get_incident_variables(expr, linear_only=True) + self.assertEqual(len(variables), 0) - expr = m.p * m.x[1] + m.x[2] - repn = generate_standard_repn(expr) - self.assertEqual(len(repn.linear_vars), 1) - self.assertIs(repn.linear_vars[0], m.x[2]) + expr = 2 * m.x[1] + 2 * m.x[2] * m.x[3] + 3 * m.x[2] + variables = self._get_incident_variables(expr, linear_only=True) + self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1]])) + + m.x[3].fix(2.5) + expr = 2 * m.x[1] + 2 * m.x[2] * m.x[3] + 3 * m.x[2] + variables = self._get_incident_variables(expr, linear_only=True) + self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1], m.x[2]])) + + +class _TestIncidenceLinearCancellation(object): + """Tests for methods that perform linear cancellation""" + + def _get_incident_variables(self, expr): + raise NotImplementedError( + "_TestIncidenceLinearCancellation should not be used directly" + ) def test_zero_coef(self): m = pyo.ConcreteModel() @@ -113,23 +125,6 @@ def test_variable_minus_itself(self): var_set = ComponentSet(variables) self.assertEqual(var_set, ComponentSet([m.x[2], m.x[3]])) - def test_linear_only(self): - m = pyo.ConcreteModel() - m.x = pyo.Var([1, 2, 3]) - - expr = 2 * m.x[1] + 4 * m.x[2] * m.x[1] - m.x[1] * pyo.exp(m.x[3]) - variables = self._get_incident_variables(expr, linear_only=True) - self.assertEqual(len(variables), 0) - - expr = 2 * m.x[1] + 2 * m.x[2] * m.x[3] + 3 * m.x[2] - variables = self._get_incident_variables(expr, linear_only=True) - self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1]])) - - m.x[3].fix(2.5) - expr = 2 * m.x[1] + 2 * m.x[2] * m.x[3] + 3 * m.x[2] - variables = self._get_incident_variables(expr, linear_only=True) - self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1], m.x[2]])) - def test_fixed_zero_linear_coefficient(self): m = pyo.ConcreteModel() m.x = pyo.Var([1, 2, 3]) @@ -148,6 +143,9 @@ def test_fixed_zero_linear_coefficient(self): variables = self._get_incident_variables(expr) self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1], m.x[2]])) + # NOTE: This test assumes that all methods that support linear cancellation + # accept a linear_only argument. If this changes, this test will need to be + # moved. def test_fixed_zero_coefficient_linear_only(self): m = pyo.ConcreteModel() m.x = pyo.Var([1, 2, 3]) @@ -159,6 +157,35 @@ def test_fixed_zero_coefficient_linear_only(self): self.assertEqual(len(variables), 1) self.assertIs(variables[0], m.x[3]) + +class TestIncidenceStandardRepn( + unittest.TestCase, + _TestIncidence, + _TestIncidenceLinearOnly, + _TestIncidenceLinearCancellation, +): + def _get_incident_variables(self, expr, **kwds): + method = IncidenceMethod.standard_repn + return get_incident_variables(expr, method=method, **kwds) + + def test_assumed_standard_repn_behavior(self): + m = pyo.ConcreteModel() + m.x = pyo.Var([1, 2]) + m.p = pyo.Param(initialize=0.0) + + # We rely on variables with constant coefficients of zero not appearing + # in the standard repn (as opposed to appearing with explicit + # coefficients of zero). + expr = m.x[1] + 0 * m.x[2] + repn = generate_standard_repn(expr) + self.assertEqual(len(repn.linear_vars), 1) + self.assertIs(repn.linear_vars[0], m.x[1]) + + expr = m.p * m.x[1] + m.x[2] + repn = generate_standard_repn(expr) + self.assertEqual(len(repn.linear_vars), 1) + self.assertIs(repn.linear_vars[0], m.x[2]) + def test_fixed_none_linear_coefficient(self): m = pyo.ConcreteModel() m.x = pyo.Var([1, 2, 3]) @@ -168,6 +195,14 @@ def test_fixed_none_linear_coefficient(self): variables = self._get_incident_variables(expr) self.assertEqual(ComponentSet(variables), ComponentSet([m.x[1], m.x[2]])) + def test_incidence_with_mutable_parameter(self): + m = pyo.ConcreteModel() + m.x = pyo.Var([1, 2, 3]) + m.p = pyo.Param(mutable=True, initialize=None) + expr = m.x[1] + m.p * m.x[1] * m.x[2] + m.x[1] * pyo.exp(m.x[3]) + variables = self._get_incident_variables(expr) + self.assertEqual(ComponentSet(variables), ComponentSet(m.x[:])) + class TestIncidenceIdentifyVariables(unittest.TestCase, _TestIncidence): def _get_incident_variables(self, expr, **kwds): @@ -192,6 +227,25 @@ def test_variable_minus_itself(self): var_set = ComponentSet(variables) self.assertEqual(var_set, ComponentSet(m.x[:])) + def test_incidence_with_mutable_parameter(self): + m = pyo.ConcreteModel() + m.x = pyo.Var([1, 2, 3]) + m.p = pyo.Param(mutable=True, initialize=None) + expr = m.x[1] + m.p * m.x[1] * m.x[2] + m.x[1] * pyo.exp(m.x[3]) + variables = self._get_incident_variables(expr) + self.assertEqual(ComponentSet(variables), ComponentSet(m.x[:])) + + +class TestIncidenceStandardRepnComputeValues( + unittest.TestCase, + _TestIncidence, + _TestIncidenceLinearOnly, + _TestIncidenceLinearCancellation, +): + def _get_incident_variables(self, expr, **kwds): + method = IncidenceMethod.standard_repn_compute_values + return get_incident_variables(expr, method=method, **kwds) + if __name__ == "__main__": unittest.main() From ecaf0530ba1c85f75afd82a3662abe639d1bf8bf Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Wed, 13 Dec 2023 13:59:26 -0700 Subject: [PATCH 11/20] re-add var_map local variable --- pyomo/contrib/incidence_analysis/interface.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pyomo/contrib/incidence_analysis/interface.py b/pyomo/contrib/incidence_analysis/interface.py index 60e77d26f7a..726398f7750 100644 --- a/pyomo/contrib/incidence_analysis/interface.py +++ b/pyomo/contrib/incidence_analysis/interface.py @@ -106,6 +106,7 @@ def get_bipartite_incidence_graph(variables, constraints, **kwds): subexpression_cache = {} subexpression_order = [] external_functions = {} + var_map = {} used_named_expressions = set() symbolic_solver_labels = False export_defined_variables = False From 9236f4f1d1d0e8b49c7cbb3da37135ab0a2ad8e8 Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Wed, 13 Dec 2023 13:59:50 -0700 Subject: [PATCH 12/20] filter duplicates from list of nonlinear vars --- pyomo/contrib/incidence_analysis/incidence.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/incidence.py b/pyomo/contrib/incidence_analysis/incidence.py index 7632f81e38a..feb8689a7c3 100644 --- a/pyomo/contrib/incidence_analysis/incidence.py +++ b/pyomo/contrib/incidence_analysis/incidence.py @@ -110,12 +110,18 @@ def _get_incident_via_ampl_repn(expr, linear_only, visitor=None): repn = visitor.walk_expression((expr, None, 0, 1.0)) nonlinear_var_ids = [] if repn.nonlinear is None else repn.nonlinear[1] - nonlinear_vars = [var_map[v_id] for v_id in nonlinear_var_ids] - nonlinear_vid_set = set(nonlinear_var_ids) + nonlinear_var_id_set = set() + unique_nonlinear_var_ids = [] + for v_id in nonlinear_var_ids: + if v_id not in nonlinear_var_id_set: + nonlinear_var_id_set.add(v_id) + unique_nonlinear_var_ids.append(v_id) + + nonlinear_vars = [var_map[v_id] for v_id in unique_nonlinear_var_ids] linear_only_vars = [ var_map[v_id] for v_id, coef in repn.linear.items() - if coef != 0.0 and v_id not in nonlinear_vid_set + if coef != 0.0 and v_id not in nonlinear_var_id_set ] if linear_only: return linear_only_vars From c04264005f6c7729b7a2054e8c7a96c389f13ef8 Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Wed, 13 Dec 2023 14:31:33 -0700 Subject: [PATCH 13/20] re-use visitor in _generate_variables_in_constraints --- pyomo/contrib/incidence_analysis/interface.py | 45 ++++++++++++++++--- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/interface.py b/pyomo/contrib/incidence_analysis/interface.py index 726398f7750..ce5f4780210 100644 --- a/pyomo/contrib/incidence_analysis/interface.py +++ b/pyomo/contrib/incidence_analysis/interface.py @@ -194,12 +194,45 @@ def extract_bipartite_subgraph(graph, nodes0, nodes1): def _generate_variables_in_constraints(constraints, **kwds): config = IncidenceConfig(kwds) - known_vars = ComponentSet() - for con in constraints: - for var in get_incident_variables(con.body, **config): - if var not in known_vars: - known_vars.add(var) - yield var + + if config.method == IncidenceMethod.ampl_repn: + subexpression_cache = {} + subexpression_order = [] + external_functions = {} + var_map = {} + used_named_expressions = set() + symbolic_solver_labels = False + export_defined_variables = False + sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) + visitor = AMPLRepnVisitor( + text_nl_template, + subexpression_cache, + subexpression_order, + external_functions, + var_map, + used_named_expressions, + symbolic_solver_labels, + export_defined_variables, + sorter, + ) + else: + visitor = None + + AMPLRepn.ActiveVisitor = visitor + try: + known_vars = ComponentSet() + for con in constraints: + for var in get_incident_variables(con.body, visitor=visitor, **config): + if var not in known_vars: + known_vars.add(var) + yield var + finally: + # NOTE: I believe this is only guaranteed to be called when the + # generator is garbage collected. This could lead to some nasty + # bug where ActiveVisitor is set for longer than we intend. + # TODO: Convert this into a function. (or yield from variables + # after this try/finally. + AMPLRepn.ActiveVisitor = None def get_structural_incidence_matrix(variables, constraints, **kwds): From 76fee13fae1bd0888dbadee083aa13d3bb437786 Mon Sep 17 00:00:00 2001 From: robbybp Date: Sat, 6 Jan 2024 15:52:52 -0700 Subject: [PATCH 14/20] move AMPLRepnVisitor construction into ConfigValue validation --- pyomo/contrib/incidence_analysis/config.py | 95 ++++++++++++++++++- pyomo/contrib/incidence_analysis/incidence.py | 42 ++------ pyomo/contrib/incidence_analysis/interface.py | 85 +++-------------- 3 files changed, 116 insertions(+), 106 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/config.py b/pyomo/contrib/incidence_analysis/config.py index db9accbddc4..31b2bd3fc22 100644 --- a/pyomo/contrib/incidence_analysis/config.py +++ b/pyomo/contrib/incidence_analysis/config.py @@ -13,6 +13,9 @@ import enum from pyomo.common.config import ConfigDict, ConfigValue, InEnum +from pyomo.common.modeling import NOTSET +from pyomo.repn.plugins.nl_writer import AMPLRepnVisitor, AMPLRepn, text_nl_template +from pyomo.repn.util import FileDeterminism, FileDeterminism_to_SortComponents class IncidenceMethod(enum.Enum): @@ -62,7 +65,92 @@ class IncidenceMethod(enum.Enum): ) -IncidenceConfig = ConfigDict() +class _ReconstructVisitor: + pass + + +def _amplrepnvisitor_validator(visitor=_ReconstructVisitor): + # This checks for and returns a valid AMPLRepnVisitor, but I don't want + # to construct this if we're not using IncidenceMethod.ampl_repn. + # It is not necessarily the end of the world if we construct this, however, + # as the code should still work. + if visitor is _ReconstructVisitor: + subexpression_cache = {} + subexpression_order = [] + external_functions = {} + var_map = {} + used_named_expressions = set() + symbolic_solver_labels = False + # TODO: Explore potential performance benefit of exporting defined variables. + # This likely only shows up if we can preserve the subexpression cache across + # multiple constraint expressions. + export_defined_variables = False + sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) + amplvisitor = AMPLRepnVisitor( + text_nl_template, + subexpression_cache, + subexpression_order, + external_functions, + var_map, + used_named_expressions, + symbolic_solver_labels, + export_defined_variables, + sorter, + ) + elif not isinstance(visitor, AMPLRepnVisitor): + raise TypeError( + "'visitor' config argument should be an instance of AMPLRepnVisitor" + ) + else: + amplvisitor = visitor + return amplvisitor + + +_ampl_repn_visitor = ConfigValue( + default=_ReconstructVisitor, + domain=_amplrepnvisitor_validator, + description="Visitor used to generate AMPLRepn of each constraint", +) + + +class _IncidenceConfigDict(ConfigDict): + + def __call__( + self, + value=NOTSET, + default=NOTSET, + domain=NOTSET, + description=NOTSET, + doc=NOTSET, + visibility=NOTSET, + implicit=NOTSET, + implicit_domain=NOTSET, + preserve_implicit=False, + ): + init_value = value + new = super().__call__( + value=value, + default=default, + domain=domain, + description=description, + doc=doc, + visibility=visibility, + implicit=implicit, + implicit_domain=implicit_domain, + preserve_implicit=preserve_implicit, + ) + + if ( + new.method == IncidenceMethod.ampl_repn + and "ampl_repn_visitor" not in init_value + ): + new.ampl_repn_visitor = _ReconstructVisitor + + return new + + + +IncidenceConfig = _IncidenceConfigDict() """Options for incidence graph generation - ``include_fixed`` -- Flag indicating whether fixed variables should be included @@ -71,6 +159,8 @@ class IncidenceMethod(enum.Enum): should be included. - ``method`` -- Method used to identify incident variables. Must be a value of the ``IncidenceMethod`` enum. +- ``ampl_repn_visitor`` -- Expression visitor used to generate ``AMPLRepn`` of each + constraint. Must be an instance of ``AMPLRepnVisitor``. """ @@ -82,3 +172,6 @@ class IncidenceMethod(enum.Enum): IncidenceConfig.declare("method", _method) + + +IncidenceConfig.declare("ampl_repn_visitor", _ampl_repn_visitor) diff --git a/pyomo/contrib/incidence_analysis/incidence.py b/pyomo/contrib/incidence_analysis/incidence.py index 62ba7a0aec7..17307e89600 100644 --- a/pyomo/contrib/incidence_analysis/incidence.py +++ b/pyomo/contrib/incidence_analysis/incidence.py @@ -80,38 +80,14 @@ def _get_incident_via_standard_repn( return unique_variables -def _get_incident_via_ampl_repn(expr, linear_only, visitor=None): - if visitor is None: - subexpression_cache = {} - subexpression_order = [] - external_functions = {} - var_map = {} - used_named_expressions = set() - symbolic_solver_labels = False - # TODO: Explore potential performance benefit of exporting defined variables. - # This likely only shows up if we can preserve the subexpression cache across - # multiple constraint expressions. - export_defined_variables = False - sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) - visitor = AMPLRepnVisitor( - text_nl_template, - subexpression_cache, - subexpression_order, - external_functions, - var_map, - used_named_expressions, - symbolic_solver_labels, - export_defined_variables, - sorter, - ) - AMPLRepn.ActiveVisitor = visitor - try: - repn = visitor.walk_expression((expr, None, 0, 1.0)) - finally: - AMPLRepn.ActiveVisitor = None - else: - var_map = visitor.var_map +def _get_incident_via_ampl_repn(expr, linear_only, visitor): + var_map = visitor.var_map + orig_activevisitor = AMPLRepn.ActiveVisitor + AMPLRepn.ActiveVisitor = visitor + try: repn = visitor.walk_expression((expr, None, 0, 1.0)) + finally: + AMPLRepn.ActiveVisitor = orig_activevisitor nonlinear_var_ids = [] if repn.nonlinear is None else repn.nonlinear[1] nonlinear_var_id_set = set() @@ -172,11 +148,11 @@ def get_incident_variables(expr, **kwds): ['x[1]', 'x[2]'] """ - visitor = kwds.pop("visitor", None) config = IncidenceConfig(kwds) method = config.method include_fixed = config.include_fixed linear_only = config.linear_only + amplrepnvisitor = config.ampl_repn_visitor if linear_only and method is IncidenceMethod.identify_variables: raise RuntimeError( "linear_only=True is not supported when using identify_variables" @@ -194,7 +170,7 @@ def get_incident_variables(expr, **kwds): expr, include_fixed, linear_only, compute_values=True ) elif method is IncidenceMethod.ampl_repn: - return _get_incident_via_ampl_repn(expr, linear_only, visitor=visitor) + return _get_incident_via_ampl_repn(expr, linear_only, amplrepnvisitor) else: raise ValueError( f"Unrecognized value {method} for the method used to identify incident" diff --git a/pyomo/contrib/incidence_analysis/interface.py b/pyomo/contrib/incidence_analysis/interface.py index ce5f4780210..b8a6c1275f9 100644 --- a/pyomo/contrib/incidence_analysis/interface.py +++ b/pyomo/contrib/incidence_analysis/interface.py @@ -93,6 +93,8 @@ def get_bipartite_incidence_graph(variables, constraints, **kwds): ``networkx.Graph`` """ + # Note that this ConfigDict contains the visitor that we will re-use + # when constructing constraints. config = IncidenceConfig(kwds) _check_unindexed(variables + constraints) N = len(variables) @@ -101,38 +103,10 @@ def get_bipartite_incidence_graph(variables, constraints, **kwds): graph.add_nodes_from(range(M), bipartite=0) graph.add_nodes_from(range(M, M + N), bipartite=1) var_node_map = ComponentMap((v, M + i) for i, v in enumerate(variables)) - - if config.method == IncidenceMethod.ampl_repn: - subexpression_cache = {} - subexpression_order = [] - external_functions = {} - var_map = {} - used_named_expressions = set() - symbolic_solver_labels = False - export_defined_variables = False - sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) - visitor = AMPLRepnVisitor( - text_nl_template, - subexpression_cache, - subexpression_order, - external_functions, - var_map, - used_named_expressions, - symbolic_solver_labels, - export_defined_variables, - sorter, - ) - else: - visitor = None - - AMPLRepn.ActiveVisitor = visitor - try: - for i, con in enumerate(constraints): - for var in get_incident_variables(con.body, visitor=visitor, **config): - if var in var_node_map: - graph.add_edge(i, var_node_map[var]) - finally: - AMPLRepn.ActiveVisitor = None + for i, con in enumerate(constraints): + for var in get_incident_variables(con.body, **config): + if var in var_node_map: + graph.add_edge(i, var_node_map[var]) return graph @@ -193,46 +167,14 @@ def extract_bipartite_subgraph(graph, nodes0, nodes1): def _generate_variables_in_constraints(constraints, **kwds): + # Note: We construct a visitor here config = IncidenceConfig(kwds) - - if config.method == IncidenceMethod.ampl_repn: - subexpression_cache = {} - subexpression_order = [] - external_functions = {} - var_map = {} - used_named_expressions = set() - symbolic_solver_labels = False - export_defined_variables = False - sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) - visitor = AMPLRepnVisitor( - text_nl_template, - subexpression_cache, - subexpression_order, - external_functions, - var_map, - used_named_expressions, - symbolic_solver_labels, - export_defined_variables, - sorter, - ) - else: - visitor = None - - AMPLRepn.ActiveVisitor = visitor - try: - known_vars = ComponentSet() - for con in constraints: - for var in get_incident_variables(con.body, visitor=visitor, **config): - if var not in known_vars: - known_vars.add(var) - yield var - finally: - # NOTE: I believe this is only guaranteed to be called when the - # generator is garbage collected. This could lead to some nasty - # bug where ActiveVisitor is set for longer than we intend. - # TODO: Convert this into a function. (or yield from variables - # after this try/finally. - AMPLRepn.ActiveVisitor = None + known_vars = ComponentSet() + for con in constraints: + for var in get_incident_variables(con.body, **config): + if var not in known_vars: + known_vars.add(var) + yield var def get_structural_incidence_matrix(variables, constraints, **kwds): @@ -329,7 +271,6 @@ class IncidenceGraphInterface(object): ``evaluate_jacobian_eq`` method instead of ``evaluate_jacobian`` rather than checking constraint expression types. - """ def __init__(self, model=None, active=True, include_inequality=True, **kwds): From e3ddb015059458899c4e1cc492dbe88d08e8a7ad Mon Sep 17 00:00:00 2001 From: robbybp Date: Sat, 6 Jan 2024 15:54:57 -0700 Subject: [PATCH 15/20] remove whitespace --- pyomo/contrib/incidence_analysis/config.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/config.py b/pyomo/contrib/incidence_analysis/config.py index 31b2bd3fc22..036c563ae75 100644 --- a/pyomo/contrib/incidence_analysis/config.py +++ b/pyomo/contrib/incidence_analysis/config.py @@ -114,7 +114,6 @@ def _amplrepnvisitor_validator(visitor=_ReconstructVisitor): class _IncidenceConfigDict(ConfigDict): - def __call__( self, value=NOTSET, @@ -149,7 +148,6 @@ def __call__( return new - IncidenceConfig = _IncidenceConfigDict() """Options for incidence graph generation From 7d64e1725b1af5173c9ff94cb885913ee1c834c2 Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Mon, 22 Jan 2024 15:10:07 -0700 Subject: [PATCH 16/20] add get_config_from_kwds to use instead of hacking ConfigDict --- pyomo/contrib/incidence_analysis/config.py | 90 +++++++++++-------- pyomo/contrib/incidence_analysis/incidence.py | 18 ++-- pyomo/contrib/incidence_analysis/interface.py | 12 +-- 3 files changed, 70 insertions(+), 50 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/config.py b/pyomo/contrib/incidence_analysis/config.py index 036c563ae75..4ab086da508 100644 --- a/pyomo/contrib/incidence_analysis/config.py +++ b/pyomo/contrib/incidence_analysis/config.py @@ -69,45 +69,16 @@ class _ReconstructVisitor: pass -def _amplrepnvisitor_validator(visitor=_ReconstructVisitor): - # This checks for and returns a valid AMPLRepnVisitor, but I don't want - # to construct this if we're not using IncidenceMethod.ampl_repn. - # It is not necessarily the end of the world if we construct this, however, - # as the code should still work. - if visitor is _ReconstructVisitor: - subexpression_cache = {} - subexpression_order = [] - external_functions = {} - var_map = {} - used_named_expressions = set() - symbolic_solver_labels = False - # TODO: Explore potential performance benefit of exporting defined variables. - # This likely only shows up if we can preserve the subexpression cache across - # multiple constraint expressions. - export_defined_variables = False - sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) - amplvisitor = AMPLRepnVisitor( - text_nl_template, - subexpression_cache, - subexpression_order, - external_functions, - var_map, - used_named_expressions, - symbolic_solver_labels, - export_defined_variables, - sorter, - ) - elif not isinstance(visitor, AMPLRepnVisitor): +def _amplrepnvisitor_validator(visitor): + if not isinstance(visitor, AMPLRepnVisitor): raise TypeError( "'visitor' config argument should be an instance of AMPLRepnVisitor" ) - else: - amplvisitor = visitor - return amplvisitor + return visitor _ampl_repn_visitor = ConfigValue( - default=_ReconstructVisitor, + default=None, domain=_amplrepnvisitor_validator, description="Visitor used to generate AMPLRepn of each constraint", ) @@ -141,14 +112,14 @@ def __call__( if ( new.method == IncidenceMethod.ampl_repn - and "ampl_repn_visitor" not in init_value + and "_ampl_repn_visitor" not in init_value ): - new.ampl_repn_visitor = _ReconstructVisitor + new._ampl_repn_visitor = _ReconstructVisitor return new -IncidenceConfig = _IncidenceConfigDict() +IncidenceConfig = ConfigDict() """Options for incidence graph generation - ``include_fixed`` -- Flag indicating whether fixed variables should be included @@ -157,8 +128,9 @@ def __call__( should be included. - ``method`` -- Method used to identify incident variables. Must be a value of the ``IncidenceMethod`` enum. -- ``ampl_repn_visitor`` -- Expression visitor used to generate ``AMPLRepn`` of each - constraint. Must be an instance of ``AMPLRepnVisitor``. +- ``_ampl_repn_visitor`` -- Expression visitor used to generate ``AMPLRepn`` of each + constraint. Must be an instance of ``AMPLRepnVisitor``. *This option is constructed + automatically when needed and should not be set by users!* """ @@ -172,4 +144,44 @@ def __call__( IncidenceConfig.declare("method", _method) -IncidenceConfig.declare("ampl_repn_visitor", _ampl_repn_visitor) +IncidenceConfig.declare("_ampl_repn_visitor", _ampl_repn_visitor) + + +def get_config_from_kwds(**kwds): + """Get an instance of IncidenceConfig from provided keyword arguments. + + If the ``method`` argument is ``IncidenceMethod.ampl_repn`` and no + ``AMPLRepnVisitor`` has been provided, a new ``AMPLRepnVisitor`` is + constructed. This function should generally be used by callers such + as ``IncidenceGraphInterface`` to ensure that a visitor is created then + re-used when calling ``get_incident_variables`` in a loop. + + """ + if ( + kwds.get("method", None) is IncidenceMethod.ampl_repn + and kwds.get("_ampl_repn_visitor", None) is None + ): + subexpression_cache = {} + subexpression_order = [] + external_functions = {} + var_map = {} + used_named_expressions = set() + symbolic_solver_labels = False + # TODO: Explore potential performance benefit of exporting defined variables. + # This likely only shows up if we can preserve the subexpression cache across + # multiple constraint expressions. + export_defined_variables = False + sorter = FileDeterminism_to_SortComponents(FileDeterminism.ORDERED) + amplvisitor = AMPLRepnVisitor( + text_nl_template, + subexpression_cache, + subexpression_order, + external_functions, + var_map, + used_named_expressions, + symbolic_solver_labels, + export_defined_variables, + sorter, + ) + kwds["_ampl_repn_visitor"] = amplvisitor + return IncidenceConfig(kwds) diff --git a/pyomo/contrib/incidence_analysis/incidence.py b/pyomo/contrib/incidence_analysis/incidence.py index 17307e89600..1fc3380fe6b 100644 --- a/pyomo/contrib/incidence_analysis/incidence.py +++ b/pyomo/contrib/incidence_analysis/incidence.py @@ -19,7 +19,9 @@ from pyomo.repn.plugins.nl_writer import AMPLRepnVisitor, AMPLRepn, text_nl_template from pyomo.repn.util import FileDeterminism, FileDeterminism_to_SortComponents from pyomo.util.subsystems import TemporarySubsystemManager -from pyomo.contrib.incidence_analysis.config import IncidenceMethod, IncidenceConfig +from pyomo.contrib.incidence_analysis.config import ( + IncidenceMethod, get_config_from_kwds +) # @@ -148,17 +150,24 @@ def get_incident_variables(expr, **kwds): ['x[1]', 'x[2]'] """ - config = IncidenceConfig(kwds) + config = get_config_from_kwds(**kwds) method = config.method include_fixed = config.include_fixed linear_only = config.linear_only - amplrepnvisitor = config.ampl_repn_visitor + amplrepnvisitor = config._ampl_repn_visitor + + # Check compatibility of arguments if linear_only and method is IncidenceMethod.identify_variables: raise RuntimeError( "linear_only=True is not supported when using identify_variables" ) if include_fixed and method is IncidenceMethod.ampl_repn: raise RuntimeError("include_fixed=True is not supported when using ampl_repn") + if method is IncidenceMethod.ampl_repn and amplrepnvisitor is None: + # Developer error, this should never happen! + raise RuntimeError("_ampl_repn_visitor must be provided when using ampl_repn") + + # Dispatch to correct method if method is IncidenceMethod.identify_variables: return _get_incident_via_identify_variables(expr, include_fixed) elif method is IncidenceMethod.standard_repn: @@ -174,6 +183,5 @@ def get_incident_variables(expr, **kwds): else: raise ValueError( f"Unrecognized value {method} for the method used to identify incident" - f" variables. Valid options are {IncidenceMethod.identify_variables}" - f" and {IncidenceMethod.standard_repn}." + f" variables. See the IncidenceMethod enum for valid methods." ) diff --git a/pyomo/contrib/incidence_analysis/interface.py b/pyomo/contrib/incidence_analysis/interface.py index b8a6c1275f9..41f0ece3a75 100644 --- a/pyomo/contrib/incidence_analysis/interface.py +++ b/pyomo/contrib/incidence_analysis/interface.py @@ -29,7 +29,7 @@ plotly, ) from pyomo.common.deprecation import deprecated -from pyomo.contrib.incidence_analysis.config import IncidenceConfig, IncidenceMethod +from pyomo.contrib.incidence_analysis.config import get_config_from_kwds from pyomo.contrib.incidence_analysis.matching import maximum_matching from pyomo.contrib.incidence_analysis.connected import get_independent_submatrices from pyomo.contrib.incidence_analysis.triangularize import ( @@ -64,7 +64,7 @@ def _check_unindexed(complist): def get_incidence_graph(variables, constraints, **kwds): - config = IncidenceConfig(kwds) + config = get_config_from_kwds(**kwds) return get_bipartite_incidence_graph(variables, constraints, **config) @@ -95,7 +95,7 @@ def get_bipartite_incidence_graph(variables, constraints, **kwds): """ # Note that this ConfigDict contains the visitor that we will re-use # when constructing constraints. - config = IncidenceConfig(kwds) + config = get_config_from_kwds(**kwds) _check_unindexed(variables + constraints) N = len(variables) M = len(constraints) @@ -168,7 +168,7 @@ def extract_bipartite_subgraph(graph, nodes0, nodes1): def _generate_variables_in_constraints(constraints, **kwds): # Note: We construct a visitor here - config = IncidenceConfig(kwds) + config = get_config_from_kwds(**kwds) known_vars = ComponentSet() for con in constraints: for var in get_incident_variables(con.body, **config): @@ -196,7 +196,7 @@ def get_structural_incidence_matrix(variables, constraints, **kwds): Entries are 1.0. """ - config = IncidenceConfig(kwds) + config = get_config_from_kwds(**kwds) _check_unindexed(variables + constraints) N, M = len(variables), len(constraints) var_idx_map = ComponentMap((v, i) for i, v in enumerate(variables)) @@ -279,7 +279,7 @@ def __init__(self, model=None, active=True, include_inequality=True, **kwds): # to cache the incidence graph for fast analysis later on. # WARNING: This cache will become invalid if the user alters their # model. - self._config = IncidenceConfig(kwds) + self._config = get_config_from_kwds(**kwds) if model is None: self._incidence_graph = None self._variables = None From 57d3134725a82f150e4b63da2f32b93ade02d201 Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Mon, 22 Jan 2024 15:11:22 -0700 Subject: [PATCH 17/20] remove now-unused ConfigDict hack --- pyomo/contrib/incidence_analysis/config.py | 39 ---------------------- 1 file changed, 39 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/config.py b/pyomo/contrib/incidence_analysis/config.py index 4ab086da508..d055be478fe 100644 --- a/pyomo/contrib/incidence_analysis/config.py +++ b/pyomo/contrib/incidence_analysis/config.py @@ -65,10 +65,6 @@ class IncidenceMethod(enum.Enum): ) -class _ReconstructVisitor: - pass - - def _amplrepnvisitor_validator(visitor): if not isinstance(visitor, AMPLRepnVisitor): raise TypeError( @@ -84,41 +80,6 @@ def _amplrepnvisitor_validator(visitor): ) -class _IncidenceConfigDict(ConfigDict): - def __call__( - self, - value=NOTSET, - default=NOTSET, - domain=NOTSET, - description=NOTSET, - doc=NOTSET, - visibility=NOTSET, - implicit=NOTSET, - implicit_domain=NOTSET, - preserve_implicit=False, - ): - init_value = value - new = super().__call__( - value=value, - default=default, - domain=domain, - description=description, - doc=doc, - visibility=visibility, - implicit=implicit, - implicit_domain=implicit_domain, - preserve_implicit=preserve_implicit, - ) - - if ( - new.method == IncidenceMethod.ampl_repn - and "_ampl_repn_visitor" not in init_value - ): - new._ampl_repn_visitor = _ReconstructVisitor - - return new - - IncidenceConfig = ConfigDict() """Options for incidence graph generation From f279fed36fe137a7d7bb33ebf48c4bd05d9f7619 Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Mon, 22 Jan 2024 15:22:42 -0700 Subject: [PATCH 18/20] split imports onto separate lines --- pyomo/contrib/incidence_analysis/incidence.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyomo/contrib/incidence_analysis/incidence.py b/pyomo/contrib/incidence_analysis/incidence.py index 1fc3380fe6b..636a400def4 100644 --- a/pyomo/contrib/incidence_analysis/incidence.py +++ b/pyomo/contrib/incidence_analysis/incidence.py @@ -20,7 +20,8 @@ from pyomo.repn.util import FileDeterminism, FileDeterminism_to_SortComponents from pyomo.util.subsystems import TemporarySubsystemManager from pyomo.contrib.incidence_analysis.config import ( - IncidenceMethod, get_config_from_kwds + IncidenceMethod, + get_config_from_kwds, ) From 227836df546cef9729f6af5dbb32664e75f3f3ac Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Fri, 9 Feb 2024 13:11:31 -0700 Subject: [PATCH 19/20] remove unused imports --- pyomo/contrib/incidence_analysis/config.py | 2 +- pyomo/contrib/incidence_analysis/incidence.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/config.py b/pyomo/contrib/incidence_analysis/config.py index d055be478fe..72d1a41ac74 100644 --- a/pyomo/contrib/incidence_analysis/config.py +++ b/pyomo/contrib/incidence_analysis/config.py @@ -14,7 +14,7 @@ import enum from pyomo.common.config import ConfigDict, ConfigValue, InEnum from pyomo.common.modeling import NOTSET -from pyomo.repn.plugins.nl_writer import AMPLRepnVisitor, AMPLRepn, text_nl_template +from pyomo.repn.plugins.nl_writer import AMPLRepnVisitor, text_nl_template from pyomo.repn.util import FileDeterminism, FileDeterminism_to_SortComponents diff --git a/pyomo/contrib/incidence_analysis/incidence.py b/pyomo/contrib/incidence_analysis/incidence.py index 636a400def4..13e9997d6c3 100644 --- a/pyomo/contrib/incidence_analysis/incidence.py +++ b/pyomo/contrib/incidence_analysis/incidence.py @@ -16,9 +16,8 @@ from pyomo.core.expr.visitor import identify_variables from pyomo.core.expr.numvalue import value as pyo_value from pyomo.repn import generate_standard_repn -from pyomo.repn.plugins.nl_writer import AMPLRepnVisitor, AMPLRepn, text_nl_template -from pyomo.repn.util import FileDeterminism, FileDeterminism_to_SortComponents from pyomo.util.subsystems import TemporarySubsystemManager +from pyomo.repn.plugins.nl_writer import AMPLRepn from pyomo.contrib.incidence_analysis.config import ( IncidenceMethod, get_config_from_kwds, From 677ebc9e67d363d364fa6f771fa610fe2bda2bbc Mon Sep 17 00:00:00 2001 From: Robert Parker Date: Fri, 9 Feb 2024 13:14:24 -0700 Subject: [PATCH 20/20] remove unused imports --- pyomo/contrib/incidence_analysis/interface.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pyomo/contrib/incidence_analysis/interface.py b/pyomo/contrib/incidence_analysis/interface.py index 41f0ece3a75..b6e6583da88 100644 --- a/pyomo/contrib/incidence_analysis/interface.py +++ b/pyomo/contrib/incidence_analysis/interface.py @@ -45,8 +45,6 @@ ) from pyomo.contrib.incidence_analysis.incidence import get_incident_variables from pyomo.contrib.pynumero.asl import AmplInterface -from pyomo.repn.plugins.nl_writer import AMPLRepnVisitor, AMPLRepn, text_nl_template -from pyomo.repn.util import FileDeterminism, FileDeterminism_to_SortComponents pyomo_nlp, pyomo_nlp_available = attempt_import( 'pyomo.contrib.pynumero.interfaces.pyomo_nlp'