Integrate new Set rewrite into Pyomo #1319

jsiirola · 2020-03-02T05:12:00Z

Fixes #N/A

Summary/Motivation:

This integrates the set rewrite (#1111 / #326) into all of Pyomo.

Changes proposed in this PR:

updating process_setarg to better handle function/generator arguments
improve simple_set_rule decorator to preserve wrapped rule API
adding indices, contains_indices methods to initializer objects
Adding TUplizeValuseInitializer to standardize tuplization of lists for multidimensional Sets
Improved deprecation support for old Set methods and attributes
adding a Set.get() method for fetchng the value stored in the set
SetOperators not connected to Blocks are always copied when cloning models/blocks
Improved implementation of DeclareGlobalSet
RealSet, IntegerSet, BinarySet moed from core.kernal.set_tyes to core.base.set. Stubs (sufficient for kernel's needs) were reimplemented in Kernel.
improved testing and handling of various edge cases encountered when converting Pyomo

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

I agree my contributions are submitted under the BSD license.
I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

Move to a more aggressive approach for immediately constructing Sets when the data is already present / defined.

We map 1-tuple indexes to scalars. Set.get() allows consumers like IndexedComponent to both test membership and retrieve the mapped value, so that the _data dictionary can match the underlying Set.

...including concrete, virtual, value and check_values()

This restores legacy functionality whereby dimensioned Set objects can be initialized from flat lists (necessary for DAT file input).

This updates Set methods so that they better match the behavior in the Python set methods they are copying.

Adding additional cases for backwards compatibility with the previous RangeSet implementation.

Constructing an abstract component that us using the class name as the component name should update the component name to the concrete version of the class.

michaelbynum

Overall, this looks great. I do have a few minor comments/questions.

pyomo/core/base/global_set.py

pyomo/core/base/set.py

Robbybp · 2020-03-10T04:02:51Z

Just merged this with my dae-tools branch and discovered a bug. When creating a SetOperator from a ContinuousSet, the _fe attribute will change from containing only the finite element points to containing the entire set. The following code illustrates:

from pyomo.environ import *
from pyomo.dae import *

m = ConcreteModel()

m.time = ContinuousSet(bounds=(0,1))
m.set1 = Set(initialize=['a','b','c'])

disc = TransformationFactory('dae.collocation')
disc.apply_to(m, wrt=m.time, nfe=2, ncp=2, scheme='LAGRANGE-RADAU')

op = m.time * m.set1

print(m.time._fe)

displaying [0, 0.166667, 0.5, 0.666667, 1], instead of [0, 0.5, 1] as in previous versions of pyomo. This is significant as it renders get_finite_elements useless. This could be caused by an erroneous call to construct, which sets self._fe = sorted(self), but I didn't see an obvious culprit in a quick scan through the code of the set operators.

Robbybp · 2020-03-10T04:19:09Z

This also seems to happen when expanding arcs.

jsiirola · 2020-03-10T14:34:08Z

@Robbybp ContinuousSet should be fixed now (including a test to catch this case in the future). Thanks!

Robbybp · 2020-03-10T18:45:12Z

@Robbybp ContinuousSet should be fixed now (including a test to catch this case in the future). Thanks!

Yep, the code that led to me finding this works now, thanks for fixing

ghackebeil · 2020-03-10T18:47:31Z

Hoping to get around to looking over this PR this week.

…

On Mar 10, 2020, at 2:45 PM, Robert Parker ***@***.***> wrote: @Robbybp ContinuousSet should be fixed now (including a test to catch this case in the future). Thanks! Yep, the code that led to me finding this works now, thanks for fixing — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

pyomo/core/base/var.py

pyomo/core/kernel/variable.py

pyomo/core/plugins/transform/util.py

pyomo/dae/diffvar.py

pyomo/core/base/set.py

pyomo/core/tests/unit/test_set.py

jsiirola · 2020-03-24T19:16:46Z

@blnicho, I think I addressed all your comments except for updating the ContinuousSet docstring.

…ng bounds

ghackebeil · 2020-03-25T19:12:40Z

examples/kernel/variables.py

@@ -1,14 +1,15 @@
 import pyomo.kernel as pmo
+import pyomo.environ as pe


I'm hoping to keep pyomo.kernel as self contained as possible until the two modeling layers are compatible. Is this import necessary?

Unfortunately, for this example., yes: Reals is a proper Pyomo Set. The alternative was to change all the Reals in the example to RealSet from kernel, and I wasn't exactly clear as to what you were trying to highlight in the example.

examples/pysp/scripting/apps/compile_scenario_tree.py

ghackebeil · 2020-03-25T19:32:40Z

pyomo/core/kernel/set_types.py

+        return False
+
+
+class BinarySet(object):


The BinarySet, BooleanSet objects don't seem to serve a purpose in kernel.

I agree, but I think I ran into examples that used BinarySet and BooleanSet from kernel. Regardless, I left them in for backwards compatibility. If we can later confirm that they are not necessary (and no one is using them), then we can delete.

I'm fairly certain this these are not needed in kernel. Binary, yes. BinarySet, no.

BooleanSet is used in pyomo/core/tests/unit/kernel/test_variable.py (and historically BooleanSet and BinarySet have been the same class), but it was not imported into pyomo.kernel.

Part of my refactoring was so that kernel proper only needs RealSet and IntegerSet, and can infer Binary from a standard get_interval() method. Longer term, I would like to standardize on BinarySet where needed so that we can re-purpose BooleanSet to be a proper Boolean (i.e., True/False, and not 0/1) set.

jsiirola

Thanks, @ghackebeil. Some responses below.

Do you see any concerns with Pyomo now having two implementations of RealSet and IntegerSet (one for core/kernel and one for core/base)? When Reals/Integers/etc got moved to be proper Set objects, this meant that we needed to support the RealSet / IntegerSet objects in both frameworks (at least for backwards compatibility).

jsiirola · 2020-03-25T19:36:37Z

examples/kernel/variables.py

@@ -1,14 +1,15 @@
 import pyomo.kernel as pmo
+import pyomo.environ as pe


Unfortunately, for this example., yes: Reals is a proper Pyomo Set. The alternative was to change all the Reals in the example to RealSet from kernel, and I wasn't exactly clear as to what you were trying to highlight in the example.

jsiirola · 2020-03-25T19:38:52Z

pyomo/core/kernel/set_types.py

+        return False
+
+
+class BinarySet(object):


I agree, but I think I ran into examples that used BinarySet and BooleanSet from kernel. Regardless, I left them in for backwards compatibility. If we can later confirm that they are not necessary (and no one is using them), then we can delete.

This restores global set (Reals, etc) imports for kernel users through the pyomo.kernel environment. Reverting changes to the kernel variables documentation and example.

jsiirola added 30 commits December 20, 2019 20:11

Adding bounds,filter, and validate support to RangeSet

5e312d2

More aggressive Set immediate construction.

7fde844

Move to a more aggressive approach for immediately constructing Sets when the data is already present / defined.

Add a Set.get() method to standardize index mapping

a4fc1a2

We map 1-tuple indexes to scalars. Set.get() allows consumers like IndexedComponent to both test membership and retrieve the mapped value, so that the _data dictionary can match the underlying Set.

Rename flatten_cross_product to subsets and make a standard Set method

d689d9f

Correct reference to class attribute

719caea

Improved handling of deprecated Set attributes/methods

b59f79d

...including concrete, virtual, value and check_values()

Set the ordered keywoard for OrderedSimpleSet and SortedSimpleSet

2db0bd9

Make SetOperator a public base class

983c663

Add legacy support for Set.add(*args)

25c7d46

Fix exge cases for @simple_set_rule

ec40b68

Clean up deprecation of set_options

89d794a

Adding TuplizeValuesInitializer to convert lists to lists of tuples

f57e0e9

This restores legacy functionality whereby dimensioned Set objects can be initialized from flat lists (necessary for DAT file input).

Removing dependence on pyutilib's flatten_tuple

f5e79b7

Raise AttributeError when iterating over non-finite Sets

0bd4d2a

Add Set.domain and (deprecated) filter attributes

096a9ef

Fix isdisjoint/issubset/issuperset/__eq__ to match set

6d7537c

This updates Set methods so that they better match the behavior in the Python set methods they are copying.

Fixing typos in documentation and exception messages

5007efc

Special case: abstract Sets with constant dimen have known dimen

2c526b0

Improve error message for non-iterable Set initializers

45bebbc

Improved automatic detection of non-finite range sets.

5531dbe

Improving RangeSet initialization

7b7e9e3

Adding additional cases for backwards compatibility with the previous RangeSet implementation.

Merge branch 'master' into set-merge

a69657a

Deprecate (but allow) passing data to SetOperator.construct()

0ca6a67

Revising global sets and reimplementing RealInterval, IntegerInterval

05f3abe

Reimplementing (and deprecating) using "in" for issubset()

5c4363b

Casting Set bounds() to int, if possible

0789100

Update implicit component names when constructing.

26dc604

Constructing an abstract component that us using the class name as the component name should update the component name to the concrete version of the class.

Adding _EmptySet class, missing Any methods

3632ef2

Fixing len/iter exceptions; comparing empty infinite RangeSets

11a0a9e

Adding missing RangeSet API methods

1925e8b

Merge branch 'master' into set-merge

cc3c924

pyomo-autotest removed the AT: RETEST label Mar 3, 2020

blnicho requested a review from ghackebeil March 3, 2020 20:22

michaelbynum requested changes Mar 4, 2020

View reviewed changes

michaelbynum approved these changes Mar 4, 2020

View reviewed changes

Updating documentation

754c43c

Fix calling construct() on a constructed ContinuousSet

ed07907

jsiirola added 4 commits March 11, 2020 08:44

Don't extract the entire stack to get the caller's frame

bf89dae

Merge branch 'master' into set-merge

dc8095f

Merge branch 'master' into set-merge

c1ff124

Fixing _ImplicitAny for new set system

350784c

blnicho reviewed Mar 24, 2020

View reviewed changes

jsiirola added 2 commits March 24, 2020 13:08

Addressing typos and review comments

2838805

Propagating optimization from is_continuous to is_binary, is_integer

a0d54b9

Updating the ContinuousSet docstring to match new behavior of enforci…

9b5b439

…ng bounds

blnicho approved these changes Mar 24, 2020

View reviewed changes

ghackebeil requested changes Mar 25, 2020

View reviewed changes

jsiirola commented Mar 25, 2020

View reviewed changes

jsiirola added 2 commits March 25, 2020 15:14

Removing unnecessary code

185cfbf

Adding AML GlobalSets to pyomo.kernel

a47ae0c

This restores global set (Reals, etc) imports for kernel users through the pyomo.kernel environment. Reverting changes to the kernel variables documentation and example.

ghackebeil approved these changes Mar 26, 2020

View reviewed changes

blnicho merged commit 9def76d into Pyomo:master Mar 31, 2020

jsiirola deleted the set-merge branch March 31, 2020 19:03

milindsmart mentioned this pull request Jul 1, 2020

Upgrade GridPath to work with Pyomo v5.7 blue-marble/gridpath#660

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate new Set rewrite into Pyomo #1319

Integrate new Set rewrite into Pyomo #1319

jsiirola commented Mar 2, 2020

michaelbynum left a comment

Robbybp commented Mar 10, 2020

Robbybp commented Mar 10, 2020

jsiirola commented Mar 10, 2020

Robbybp commented Mar 10, 2020

ghackebeil commented Mar 10, 2020 via email

jsiirola commented Mar 24, 2020

ghackebeil Mar 25, 2020

jsiirola Mar 25, 2020 •

edited

Loading

ghackebeil Mar 25, 2020

jsiirola Mar 25, 2020

ghackebeil Mar 25, 2020

jsiirola Mar 25, 2020

jsiirola left a comment

jsiirola Mar 25, 2020 •

edited

Loading

jsiirola Mar 25, 2020

		@@ -1,14 +1,15 @@
		import pyomo.kernel as pmo
		import pyomo.environ as pe

Integrate new Set rewrite into Pyomo #1319

Integrate new Set rewrite into Pyomo #1319

Conversation

jsiirola commented Mar 2, 2020

Fixes #N/A

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

michaelbynum left a comment

Choose a reason for hiding this comment

Robbybp commented Mar 10, 2020

Robbybp commented Mar 10, 2020

jsiirola commented Mar 10, 2020

Robbybp commented Mar 10, 2020

ghackebeil commented Mar 10, 2020 via email

jsiirola commented Mar 24, 2020

ghackebeil Mar 25, 2020

Choose a reason for hiding this comment

jsiirola Mar 25, 2020 • edited Loading

Choose a reason for hiding this comment

ghackebeil Mar 25, 2020

Choose a reason for hiding this comment

jsiirola Mar 25, 2020

Choose a reason for hiding this comment

ghackebeil Mar 25, 2020

Choose a reason for hiding this comment

jsiirola Mar 25, 2020

Choose a reason for hiding this comment

jsiirola left a comment

Choose a reason for hiding this comment

jsiirola Mar 25, 2020 • edited Loading

Choose a reason for hiding this comment

jsiirola Mar 25, 2020

Choose a reason for hiding this comment

jsiirola Mar 25, 2020 •

edited

Loading

jsiirola Mar 25, 2020 •

edited

Loading