From 52fe9f0b90f191753d0ff08da224f62643af2a1d Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 08:17:56 -0600 Subject: [PATCH 1/7] LP: warn for unused export suffixes --- pyomo/repn/plugins/lp_writer.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pyomo/repn/plugins/lp_writer.py b/pyomo/repn/plugins/lp_writer.py index 5f3972abce6..d486b707765 100644 --- a/pyomo/repn/plugins/lp_writer.py +++ b/pyomo/repn/plugins/lp_writer.py @@ -320,6 +320,27 @@ def write(self, model): timer.toc('Initialized column order', level=logging.DEBUG) + # We don't export any suffix information to the LP file + # + if component_map[Suffix]: + suffixesByName = {} + for block in component_map[Suffix]: + for suffix in block.component_objects( + Suffix, active=True, descend_into=False, sort=sorter + ): + if not (suffix.direction & Suffix.EXPORT): + continue + if suffix.name in suffixesByName: + suffixesByName[suffix.name].append(suffix) + else: + suffixesByName[suffix.name] = [suffix] + for name, suffixes in suffixesByName.items(): + logger.warning( + f"EXPORT Suffix {name} found on {len(suffixes)} blocks:\n\t" + + "\n\t".join(s.name for s in suffixes) + + "LP writer cannot export suffixes to LP file. Skipping." + ) + ostream.write(f"\\* Source Pyomo model name={model.name} *\\\n\n") # From 8e6b11c68c978db1179162e87f1a65bced773e46 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 08:41:30 -0600 Subject: [PATCH 2/7] Ignore empty/import suffixes --- pyomo/repn/plugins/lp_writer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyomo/repn/plugins/lp_writer.py b/pyomo/repn/plugins/lp_writer.py index d486b707765..5275f300ed6 100644 --- a/pyomo/repn/plugins/lp_writer.py +++ b/pyomo/repn/plugins/lp_writer.py @@ -328,7 +328,7 @@ def write(self, model): for suffix in block.component_objects( Suffix, active=True, descend_into=False, sort=sorter ): - if not (suffix.direction & Suffix.EXPORT): + if not suffix.export_enabled() or not suffix: continue if suffix.name in suffixesByName: suffixesByName[suffix.name].append(suffix) From 6bcea522721607e2fb922e75a2e76eb7194aa962 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 08:42:12 -0600 Subject: [PATCH 3/7] report suffixes grouped by local_name --- pyomo/repn/plugins/lp_writer.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pyomo/repn/plugins/lp_writer.py b/pyomo/repn/plugins/lp_writer.py index 5275f300ed6..309afa8ae70 100644 --- a/pyomo/repn/plugins/lp_writer.py +++ b/pyomo/repn/plugins/lp_writer.py @@ -330,10 +330,11 @@ def write(self, model): ): if not suffix.export_enabled() or not suffix: continue - if suffix.name in suffixesByName: - suffixesByName[suffix.name].append(suffix) + name = suffix.local_name + if name in suffixesByName: + suffixesByName[name].append(suffix) else: - suffixesByName[suffix.name] = [suffix] + suffixesByName[name] = [suffix] for name, suffixes in suffixesByName.items(): logger.warning( f"EXPORT Suffix {name} found on {len(suffixes)} blocks:\n\t" From e936e5c47783ed799be63766ba12a31e60c686a7 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 08:42:30 -0600 Subject: [PATCH 4/7] Improve suffix warning message --- pyomo/repn/plugins/lp_writer.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pyomo/repn/plugins/lp_writer.py b/pyomo/repn/plugins/lp_writer.py index 309afa8ae70..fab94d313d5 100644 --- a/pyomo/repn/plugins/lp_writer.py +++ b/pyomo/repn/plugins/lp_writer.py @@ -336,10 +336,12 @@ def write(self, model): else: suffixesByName[name] = [suffix] for name, suffixes in suffixesByName.items(): + n = len(suffixes) + plural = 's' if n > 1 else '' logger.warning( - f"EXPORT Suffix {name} found on {len(suffixes)} blocks:\n\t" - + "\n\t".join(s.name for s in suffixes) - + "LP writer cannot export suffixes to LP file. Skipping." + f"EXPORT Suffix '{name}' found on {n} block{plural}:\n " + + "\n ".join(s.name for s in suffixes) + + "\nLP writer cannot export suffixes to LP files. Skipping." ) ostream.write(f"\\* Source Pyomo model name={model.name} *\\\n\n") From c68a9f5775d6a80386ecb77a41d286b52436e184 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 08:42:47 -0600 Subject: [PATCH 5/7] Add tests --- pyomo/repn/tests/cpxlp/test_lpv2.py | 68 +++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 pyomo/repn/tests/cpxlp/test_lpv2.py diff --git a/pyomo/repn/tests/cpxlp/test_lpv2.py b/pyomo/repn/tests/cpxlp/test_lpv2.py new file mode 100644 index 00000000000..8ec0582acc2 --- /dev/null +++ b/pyomo/repn/tests/cpxlp/test_lpv2.py @@ -0,0 +1,68 @@ +# ___________________________________________________________________________ +# +# Pyomo: Python Optimization Modeling Objects +# Copyright (c) 2008-2022 +# National Technology and Engineering Solutions of Sandia, LLC +# Under the terms of Contract DE-NA0003525 with National Technology and +# Engineering Solutions of Sandia, LLC, the U.S. Government retains certain +# rights in this software. +# This software is distributed under the 3-clause BSD License. +# ___________________________________________________________________________ + +from io import StringIO + +import pyomo.common.unittest as unittest + +from pyomo.common.log import LoggingIntercept +from pyomo.environ import ConcreteModel, Block, Constraint, Var, Objective, Suffix + +from pyomo.repn.plugins.lp_writer import LPWriter + +class TestLPv2(unittest.TestCase): + def test_warn_export_suffixes(self): + m = ConcreteModel() + m.x = Var() + m.obj = Objective(expr=m.x) + m.con = Constraint(expr=m.x>=2) + m.b = Block() + m.ignored = Suffix(direction=Suffix.IMPORT) + m.duals = Suffix(direction=Suffix.IMPORT_EXPORT) + m.b.duals = Suffix(direction=Suffix.IMPORT_EXPORT) + m.b.scaling = Suffix(direction=Suffix.EXPORT) + + # Empty suffixes are ignored + writer = LPWriter() + with LoggingIntercept() as LOG: + writer.write(m, StringIO()) + self.assertEqual(LOG.getvalue(), "") + + # Import are ignored, export and import/export are warned + m.duals[m.con] = 5 + m.ignored[m.x] = 6 + m.b.scaling[m.x] = 7 + + writer = LPWriter() + with LoggingIntercept() as LOG: + writer.write(m, StringIO()) + self.assertEqual(LOG.getvalue(), """EXPORT Suffix 'duals' found on 1 block: + duals +LP writer cannot export suffixes to LP files. Skipping. +EXPORT Suffix 'scaling' found on 1 block: + b.scaling +LP writer cannot export suffixes to LP files. Skipping. +""") + + # Counting works correctly + m.b.duals[m.x] = 7 + + writer = LPWriter() + with LoggingIntercept() as LOG: + writer.write(m, StringIO()) + self.assertEqual(LOG.getvalue(), """EXPORT Suffix 'duals' found on 2 blocks: + duals + b.duals +LP writer cannot export suffixes to LP files. Skipping. +EXPORT Suffix 'scaling' found on 1 block: + b.scaling +LP writer cannot export suffixes to LP files. Skipping. +""") From c9b2252038f11a091dfc1bddac49173caeb1c2d6 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 09:51:41 -0600 Subject: [PATCH 6/7] Apply black --- pyomo/repn/tests/cpxlp/test_lpv2.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pyomo/repn/tests/cpxlp/test_lpv2.py b/pyomo/repn/tests/cpxlp/test_lpv2.py index 8ec0582acc2..fbe4f15fbe9 100644 --- a/pyomo/repn/tests/cpxlp/test_lpv2.py +++ b/pyomo/repn/tests/cpxlp/test_lpv2.py @@ -18,18 +18,19 @@ from pyomo.repn.plugins.lp_writer import LPWriter + class TestLPv2(unittest.TestCase): def test_warn_export_suffixes(self): m = ConcreteModel() m.x = Var() m.obj = Objective(expr=m.x) - m.con = Constraint(expr=m.x>=2) + m.con = Constraint(expr=m.x >= 2) m.b = Block() m.ignored = Suffix(direction=Suffix.IMPORT) m.duals = Suffix(direction=Suffix.IMPORT_EXPORT) m.b.duals = Suffix(direction=Suffix.IMPORT_EXPORT) m.b.scaling = Suffix(direction=Suffix.EXPORT) - + # Empty suffixes are ignored writer = LPWriter() with LoggingIntercept() as LOG: @@ -44,13 +45,16 @@ def test_warn_export_suffixes(self): writer = LPWriter() with LoggingIntercept() as LOG: writer.write(m, StringIO()) - self.assertEqual(LOG.getvalue(), """EXPORT Suffix 'duals' found on 1 block: + self.assertEqual( + LOG.getvalue(), + """EXPORT Suffix 'duals' found on 1 block: duals LP writer cannot export suffixes to LP files. Skipping. EXPORT Suffix 'scaling' found on 1 block: b.scaling LP writer cannot export suffixes to LP files. Skipping. -""") +""", + ) # Counting works correctly m.b.duals[m.x] = 7 @@ -58,11 +62,14 @@ def test_warn_export_suffixes(self): writer = LPWriter() with LoggingIntercept() as LOG: writer.write(m, StringIO()) - self.assertEqual(LOG.getvalue(), """EXPORT Suffix 'duals' found on 2 blocks: + self.assertEqual( + LOG.getvalue(), + """EXPORT Suffix 'duals' found on 2 blocks: duals b.duals LP writer cannot export suffixes to LP files. Skipping. EXPORT Suffix 'scaling' found on 1 block: b.scaling LP writer cannot export suffixes to LP files. Skipping. -""") +""", + ) From e228786d0671c7b21fadbb83a41db638269303ef Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 10:21:36 -0600 Subject: [PATCH 7/7] Fix kernel/aml inconsistency: make {export,import}_nabled a method --- pyomo/core/kernel/suffix.py | 6 ++---- pyomo/core/tests/unit/kernel/test_suffix.py | 16 ++++++++-------- pyomo/solvers/tests/models/base.py | 14 ++------------ 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/pyomo/core/kernel/suffix.py b/pyomo/core/kernel/suffix.py index 0416c1269f9..77079364703 100644 --- a/pyomo/core/kernel/suffix.py +++ b/pyomo/core/kernel/suffix.py @@ -102,13 +102,11 @@ def __init__(self, *args, **kwds): # Interface # - @property def export_enabled(self): """Returns :const:`True` when this suffix is enabled for export to solvers.""" return bool(self._direction & suffix.EXPORT) - @property def import_enabled(self): """Returns :const:`True` when this suffix is enabled for import from solutions.""" @@ -233,7 +231,7 @@ def export_suffix_generator(blk, datatype=_noarg, active=True, descend_into=True """ for suf in filter( lambda x: ( - x.export_enabled and ((datatype is _noarg) or (x.datatype is datatype)) + x.export_enabled() and ((datatype is _noarg) or (x.datatype is datatype)) ), blk.components(ctype=suffix._ctype, active=active, descend_into=descend_into), ): @@ -267,7 +265,7 @@ def import_suffix_generator(blk, datatype=_noarg, active=True, descend_into=True """ for suf in filter( lambda x: ( - x.import_enabled and ((datatype is _noarg) or (x.datatype is datatype)) + x.import_enabled() and ((datatype is _noarg) or (x.datatype is datatype)) ), blk.components(ctype=suffix._ctype, active=active, descend_into=descend_into), ): diff --git a/pyomo/core/tests/unit/kernel/test_suffix.py b/pyomo/core/tests/unit/kernel/test_suffix.py index 6565c6a920d..c4c75278d50 100644 --- a/pyomo/core/tests/unit/kernel/test_suffix.py +++ b/pyomo/core/tests/unit/kernel/test_suffix.py @@ -111,20 +111,20 @@ def test_import_export_enabled(self): s = suffix() s.direction = suffix.LOCAL self.assertEqual(s.direction, suffix.LOCAL) - self.assertEqual(s.export_enabled, False) - self.assertEqual(s.import_enabled, False) + self.assertEqual(s.export_enabled(), False) + self.assertEqual(s.import_enabled(), False) s.direction = suffix.IMPORT self.assertEqual(s.direction, suffix.IMPORT) - self.assertEqual(s.export_enabled, False) - self.assertEqual(s.import_enabled, True) + self.assertEqual(s.export_enabled(), False) + self.assertEqual(s.import_enabled(), True) s.direction = suffix.EXPORT self.assertEqual(s.direction, suffix.EXPORT) - self.assertEqual(s.export_enabled, True) - self.assertEqual(s.import_enabled, False) + self.assertEqual(s.export_enabled(), True) + self.assertEqual(s.import_enabled(), False) s.direction = suffix.IMPORT_EXPORT self.assertEqual(s.direction, suffix.IMPORT_EXPORT) - self.assertEqual(s.export_enabled, True) - self.assertEqual(s.import_enabled, True) + self.assertEqual(s.export_enabled(), True) + self.assertEqual(s.import_enabled(), True) with self.assertRaises(ValueError): s.direction = 'export' diff --git a/pyomo/solvers/tests/models/base.py b/pyomo/solvers/tests/models/base.py index 5adfae8c729..106e8860145 100644 --- a/pyomo/solvers/tests/models/base.py +++ b/pyomo/solvers/tests/models/base.py @@ -142,12 +142,7 @@ def save_current_solution(self, filename, **kwds): (suffix, getattr(model, suffix)) for suffix in kwds.pop('suffixes', []) ) for suf in suffixes.values(): - if isinstance(self.model, IBlock): - assert isinstance(suf, pmo.suffix) - assert suf.import_enabled - else: - assert isinstance(suf, Suffix) - assert suf.import_enabled() + assert suf.import_enabled() with open(filename, 'w') as f: # @@ -197,12 +192,7 @@ def validate_current_solution(self, **kwds): ) exclude = kwds.pop('exclude_suffixes', set()) for suf in suffixes.values(): - if isinstance(self.model, IBlock): - assert isinstance(suf, pmo.suffix) - assert suf.import_enabled - else: - assert isinstance(suf, Suffix) - assert suf.import_enabled() + assert suf.import_enabled() solution = None error_str = ( "Difference in solution for {0}.{1}:\n\tBaseline - {2}\n\tCurrent - {3}"