From 2830e212e2470e1d968cd5da4ef2ef48a4eab814 Mon Sep 17 00:00:00 2001
From: Ioannis Filippidis <jfilippidis@gmail.com>
Date: Sun, 14 Nov 2021 19:00:07 +0100
Subject: [PATCH] ENH: support roots as `dict` in Pickle interface

- API: Previously, the returned `dict` was a mapping from
  dumped to all loaded BDD nodes, whereas now it is
  a mapping from names (as strings) to loaded roots
  (BDD node references).

The change of meaning for the returned value
is not an issue for files dumped from earlier `dd`
versions, because attempting to load a BDD from
a Pickle file that was dumped with an earlier version
of `dd` would raise an error,
specifically when attempting to access the key `'root'`
of the unpickled `dict` (i.e., `d['roots']`).

This API change *is* an issue for files dumped
with this version of `dd`, in case the callee of
the method `BDD.load()` assumes the old interface
of the methods:

- `dd.bdd.BDD.load()`
- `dd.autoref.BDD.load()`.

There is no way to detect this case.

- REF: change `dd.cudd.BDD.dump()` to `def` (was `cpdef`)
  to use closures (for the variable `cache`).
---
 dd/autoref.py | 18 ++++++++++++------
 dd/bdd.py     | 19 +++++++++++++------
 dd/cudd.pyx   | 21 +++++++++++++++------
 3 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/dd/autoref.py b/dd/autoref.py
index f2eb7c9f..bedf9aaf 100644
--- a/dd/autoref.py
+++ b/dd/autoref.py
@@ -10,6 +10,7 @@
 
 from dd import _abc
 from dd import _copy
+import dd._utils as _utils
 from dd import bdd as _bdd
 
 
@@ -262,11 +263,16 @@ def dump(self, filename, roots=None,
                     f'name "{filename}"')
         if filetype == 'json':
             _copy.dump_json(roots, filename)
-        else:
-            if roots is not None:
-                roots = [u.node for u in roots]
-            self._bdd.dump(filename, roots=roots,
-                           filetype=filetype)
+            return
+        if roots is not None:
+            def mapper(u):
+                return u.node
+            roots = _utils._map_container(
+                mapper, roots)
+        self._bdd.dump(
+            filename,
+            roots=roots,
+            filetype=filetype)
 
     def load(self, filename, levels=True):
         """Load nodes from Pickle or JSON file `filename`.
@@ -298,7 +304,7 @@ def load(self, filename, levels=True):
 
     def _load_pickle(self, filename, levels=True):
         roots = self._bdd.load(filename, levels=levels)
-        return list(map(self._wrap, roots))
+        return _utils._map_container(self._wrap, roots)
 
     def assert_consistent(self):
         self._bdd.assert_consistent()
diff --git a/dd/bdd.py b/dd/bdd.py
index 444c29f0..838eb527 100644
--- a/dd/bdd.py
+++ b/dd/bdd.py
@@ -48,6 +48,7 @@
 
 import dd._abc
 import dd._parser
+import dd._utils as _utils
 # inline:
 # import networkx
 # import pydot
@@ -1575,7 +1576,8 @@ def _dump_bdd(self, roots, filename, **kw):
         if roots is None:
             nodes = self._succ
         else:
-            nodes = self.descendants(roots)
+            values = _utils._values_of(roots)
+            nodes = self.descendants(values)
         succ = ((k, self._succ[k]) for k in nodes)
         d = dict(
             vars=self.vars,
@@ -1587,13 +1589,18 @@ def _dump_bdd(self, roots, filename, **kw):
 
     def load(self, filename, levels=True):
         name = filename.lower()
-        if name.endswith('.p'):
-            umap, roots = self._load_pickle(
-                filename, levels=levels)
-            return [umap[u] for u in roots]
-        else:
+        if not name.endswith('.p'):
             raise ValueError(
                 f'Unknown file type of "{filename}"')
+        umap, roots = self._load_pickle(
+            filename, levels=levels)
+        def map_node(u):
+            v = umap[abs(u)]
+            if u < 0:
+                return - v
+            else:
+                return v
+        return _utils._map_container(map_node, roots)
 
     def _load_pickle(self, filename, levels=True):
         with open(filename, 'rb') as f:
diff --git a/dd/cudd.pyx b/dd/cudd.pyx
index 463a9da9..a5c3679d 100644
--- a/dd/cudd.pyx
+++ b/dd/cudd.pyx
@@ -1791,7 +1791,7 @@ cdef class BDD:
             expr = f'(~ {expr})'
         return expr
 
-    cpdef dump(
+    def dump(
             self, filename, roots,
             filetype=None):
         """Write BDDs to `filename`.
@@ -1851,12 +1851,21 @@ cdef class BDD:
             return self._dump_dddmp(u, filename)
         elif filetype == 'json':
             return _copy.dump_json(roots, filename)
+        bdd = autoref.BDD()
+        _copy.copy_vars(self, bdd)
+            # preserve levels
+        if roots is None:
+            root_nodes = None
         else:
-            bdd = autoref.BDD()
-            _copy.copy_vars(self, bdd)
-                # preserve levels
-            v = _copy.copy_bdds_from(roots, bdd)
-            bdd.dump(filename, v, filetype=filetype)
+            cache = dict()
+            def mapper(u):
+                return _copy.copy_bdd(
+                    u, bdd, cache)
+            root_nodes = _utils._map_container(
+                mapper, roots)
+        bdd.dump(
+            filename, root_nodes,
+            filetype=filetype)
 
     cpdef _dump_dddmp(self, Function u, fname):
         """Dump BDD as DDDMP file `fname`."""