Merge pull request #3597 from jsiirola/deepcopy-results

Resolve issues copying and pickling `SolverResults`

Author: blnicho

pyomo/opt/results/container.py

@@ -9,12 +9,11 @@
 #  This software is distributed under the 3-clause BSD License.
 #  ___________________________________________________________________________
 
-import copy
 import enum
 from io import StringIO
 from math import inf
 
-from pyomo.common.collections import Bunch
+from pyomo.common.collections import Bunch, Sequence, Mapping
 
 
 class ScalarType(str, enum.Enum):
@@ -35,17 +34,31 @@ def __str__(self):
 
 
 default_print_options = Bunch(schema=False, ignore_time=False)
-
 strict = False
 
 
 class UndefinedData(object):
+    singleton = {}
+
+    def __new__(cls, name='undefined'):
+        if name not in UndefinedData.singleton:
+            UndefinedData.singleton[name] = super().__new__(cls)
+            UndefinedData.singleton[name].name = name
+        return UndefinedData.singleton[name]
+
+    def __deepcopy__(self, memo):
+        # Prevent deepcopy from duplicating this object
+        return self
+
+    def __reduce__(self):
+        return self.__class__, (self.name,)
+
     def __str__(self):
-        return "<undefined>"
+        return f"<{self.name}>"
 
 
-undefined = UndefinedData()
-ignore = UndefinedData()
+undefined = UndefinedData('undefined')
+ignore = UndefinedData('ignore')
 
 
 class ScalarData(object):
@@ -64,6 +77,10 @@ def __init__(
         self.scalar_description = scalar_description
         self.scalar_type = type
         self._required = required
+        self._active = False
+
+    def __eq__(self, other):
+        return self.__dict__ == other.__dict__
 
     def get_value(self):
         if isinstance(self.value, enum.Enum):
@@ -109,9 +126,9 @@ def pprint(self, ostream, option, prefix="", repn=None):
 
         value = self.yaml_fix(self.get_value())
 
-        if value is inf:
+        if value == inf:
             value = '.inf'
-        elif value is -inf:
+        elif value == -inf:
             value = '-.inf'
 
         if not option.schema and self.description is None and self.units is None:
@@ -149,8 +166,8 @@ def yaml_fix(self, val):
 
     def load(self, repn):
         if type(repn) is dict:
-            for key in repn:
-                setattr(self, key, repn[key])
+            for key, val in repn.items():
+                setattr(self, key, val)
         else:
             self.value = repn
 
@@ -167,12 +184,15 @@ def __init__(self, cls):
 
     def __len__(self):
         if '_list' in self.__dict__:
-            return len(self.__dict__['_list'])
+            return len(self._list)
         return 0
 
     def __getitem__(self, i):
         return self._list[i]
 
+    def __eq__(self, other):
+        return self.__dict__ == other.__dict__
+
     def clear(self):
         self._list = []
 
@@ -183,21 +203,15 @@ def __call__(self, i=0):
         return self._list[i]
 
     def __getattr__(self, name):
-        try:
-            return self.__dict__[name]
-        except:
-            pass
+        if name[0] == "_":
+            super().__getattr__(name)
         if len(self) == 0:
             self.add()
         return getattr(self._list[0], name)
 
     def __setattr__(self, name, val):
-        if name == "__class__":
-            self.__class__ = val
-            return
         if name[0] == "_":
-            self.__dict__[name] = val
-            return
+            return super().__setattr__(name, val)
         if len(self) == 0:
             self.add()
         setattr(self._list[0], name, val)
@@ -239,16 +253,10 @@ def load(self, repn):
             item = self.add()
             item.load(data)
 
-    def __getstate__(self):
-        return copy.copy(self.__dict__)
-
-    def __setstate__(self, state):
-        self.__dict__.update(state)
-
     def __str__(self):
         ostream = StringIO()
         option = default_print_options
-        self.pprint(ostream, self._option, repn=self._repn_(self._option))
+        self.pprint(ostream, option, repn=self._repn_(option))
         return ostream.getvalue()
 
 
@@ -259,41 +267,21 @@ def __str__(self):
 # first letter is capitalized.
 #
 class MapContainer(dict):
-    def __getnewargs_ex__(self):
-        # Pass arguments to __new__ when unpickling
-        return ((0, 0), {})
-
-    def __getnewargs__(self):
-        # Pass arguments to __new__ when unpickling
-        return (0, 0)
-
-    def __new__(cls, *args, **kwargs):
-        #
-        # If the user provides "too many" arguments, then
-        # pre-initialize the '_order' attribute.  This pre-initializes
-        # the class during unpickling.
-        #
-        _instance = super(MapContainer, cls).__new__(cls, *args, **kwargs)
-        if len(args) > 1:
-            super(MapContainer, _instance).__setattr__('_order', [])
-        return _instance
 
     def __init__(self, ordered=False):
-        dict.__init__(self)
+        super().__init__()
         self._active = True
         self._required = False
-        self._ordered = ordered
-        self._order = []
         self._option = default_print_options
 
-    def keys(self):
-        return self._order
+    def __eq__(self, other):
+        # We need to check both our __dict__ (local attributes) and the
+        # underlying dict data (which doesn't show up in the __dict__).
+        # So we will use the base __eq__ in addition to checking
+        # __dict__.
+        return super().__eq__(other) and self.__dict__ == other.__dict__
 
     def __getattr__(self, name):
-        try:
-            return self.__dict__[name]
-        except:
-            pass
         try:
             self._active = True
             return self[self._convert(name)]
@@ -307,12 +295,8 @@ def __getattr__(self, name):
         )
 
     def __setattr__(self, name, val):
-        if name == "__class__":
-            self.__class__ = val
-            return
         if name[0] == "_":
-            self.__dict__[name] = val
-            return
+            return super().__setattr__(name, val)
         self._active = True
         tmp = self._convert(name)
         if tmp not in self:
@@ -341,12 +325,18 @@ def __setitem__(self, name, val):
         self._set_value(tmp, val)
 
     def _set_value(self, name, val):
-        if isinstance(val, ListContainer) or isinstance(val, MapContainer):
-            dict.__setitem__(self, name, val)
+        if isinstance(val, (ListContainer, MapContainer)):
+            super().__setitem__(name, val)
         elif isinstance(val, ScalarData):
-            dict.__getitem__(self, name).value = val.value
+            data = super().__getitem__(name)
+            data.value = val.value
+            data._active = val._active
+            data._required = val._required
+            data.scalar_type = val.scalar_type
         else:
-            dict.__getitem__(self, name).value = val
+            data = super().__getitem__(name)
+            data.value = val
+            data._active = True
 
     def __getitem__(self, name):
         tmp = self._convert(name)
@@ -357,25 +347,21 @@ def __getitem__(self, name):
                 + "' for object with type "
                 + str(type(self))
             )
-        item = dict.__getitem__(self, tmp)
-        if isinstance(item, ListContainer) or isinstance(item, MapContainer):
+        item = super().__getitem__(tmp)
+        if isinstance(item, (ListContainer, MapContainer)):
             return item
         return item.value
 
     def declare(self, name, **kwds):
         if name in self or type(name) is int:
             return
-        tmp = self._convert(name)
-        self._order.append(tmp)
-        if 'value' in kwds and (
-            isinstance(kwds['value'], MapContainer)
-            or isinstance(kwds['value'], ListContainer)
-        ):
+        data = kwds.get('value', None)
+        if isinstance(data, (MapContainer, ListContainer)):
             if 'active' in kwds:
-                kwds['value']._active = kwds['active']
+                data._active = kwds['active']
             if 'required' in kwds and kwds['required'] is True:
-                kwds['value']._required = True
-            dict.__setitem__(self, tmp, kwds['value'])
+                data._required = True
+            super().__setitem__(self._convert(name), data)
         else:
             data = ScalarData(**kwds)
             if 'required' in kwds and kwds['required'] is True:
@@ -387,23 +373,16 @@ def declare(self, name, **kwds):
             #
             # if 'value' in kwds:
             #    data._default = kwds['value']
-            dict.__setitem__(self, tmp, data)
+            super().__setitem__(self._convert(name), data)
 
     def _repn_(self, option):
         if not option.schema and not self._active and not self._required:
             return ignore
-        if self._ordered:
-            tmp = []
-            for key in self._order:
-                rep = dict.__getitem__(self, key)._repn_(option)
-                if not rep == ignore:
-                    tmp.append({key: rep})
-        else:
-            tmp = {}
-            for key in self.keys():
-                rep = dict.__getitem__(self, key)._repn_(option)
-                if not rep == ignore:
-                    tmp[key] = rep
+        tmp = {}
+        for key, val in self.items():
+            rep = val._repn_(option)
+            if not rep == ignore:
+                tmp[key] = rep
         return tmp
 
     def _convert(self, name):
@@ -417,7 +396,6 @@ def __repr__(self):
 
     def __str__(self):
         ostream = StringIO()
-        option = default_print_options
         self.pprint(ostream, self._option, repn=self._repn_(self._option))
         return ostream.getvalue()
 
@@ -427,10 +405,9 @@ def pprint(self, ostream, option, from_list=False, prefix="", repn=None):
         else:
             _prefix = prefix
             ostream.write('\n')
-        for key in self._order:
+        for key, item in self.items():
             if not key in repn:
                 continue
-            item = dict.__getitem__(self, key)
             ostream.write(_prefix + key + ": ")
             _prefix = prefix
             if isinstance(item, ListContainer):
@@ -439,46 +416,16 @@ def pprint(self, ostream, option, from_list=False, prefix="", repn=None):
                 item.pprint(ostream, option, prefix=_prefix + "  ", repn=repn[key])
 
     def load(self, repn):
-        for key in repn:
+        for key, val in repn.items():
             tmp = self._convert(key)
             if tmp not in self:
                 self.declare(tmp)
-            item = dict.__getitem__(self, tmp)
+            item = super().__getitem__(tmp)
             item._active = True
-            item.load(repn[key])
-
-    def __getnewargs__(self):
-        return (False, False)
-
-    def __getstate__(self):
-        return copy.copy(self.__dict__)
-
-    def __setstate__(self, state):
-        self.__dict__.update(state)
-
-
-if __name__ == '__main__':
-    d = MapContainer()
-    d.declare('f')
-    d.declare('g')
-    d.declare('h')
-    d.declare('i', value=ListContainer(UndefinedData))
-    d.declare('j', value=ListContainer(UndefinedData), active=False)
-    print("X")
-    d.f = 1
-    print("Y")
-    print(d.f)
-    print(d.keys())
-    d.g = None
-    print(d.keys())
-    try:
-        print(d.f, d.g, d.h)
-    except:
-        pass
-    d['h'] = None
-    print("")
-    print("FINAL")
-    print(d.f, d.g, d.h, d.i, d.j)
-    print(d.i._active, d.j._active)
-    d.j.add()
-    print(d.i._active, d.j._active)
+            item.load(val)
+
+
+# Register these as sequence / mapping types (so things like
+# assertStructuredAlmostEqual will process them correctly)
+Sequence.register(ListContainer)
+Mapping.register(MapContainer)

pyomo/opt/results/problem.py

@@ -24,7 +24,7 @@ def __str__(self):
 
 class ProblemInformation(MapContainer):
     def __init__(self):
-        MapContainer.__init__(self)
+        super().__init__()
         self.declare('name')
         self.declare('lower_bound', value=float('-inf'))
         self.declare('upper_bound', value=float('inf'))