Merge pull request #53 from arcondello/fix/to_networkx

Fix `Symbol.to_networkx()`

dwave/optimization/model.pyx

@@ -40,6 +40,7 @@ import numpy as np
 from cpython cimport Py_buffer
 from cython.operator cimport dereference as deref, preincrement as inc
 from cython.operator cimport typeid
+from libc.stdint cimport uintptr_t
 from libcpp cimport bool
 from libcpp.utility cimport move
 from libcpp.vector cimport vector
@@ -845,53 +846,71 @@ cdef class Model:
     def to_networkx(self):
         """Convert the model to a NetworkX graph.
 
-        Note:
-            Currently requires the installation of a GNU compiler.
-
         Returns:
-            A :obj:`NetworkX <networkx:networkx.Graph>` graph.
+            A :obj:`NetworkX <networkx:networkx.MultiDiGraph>` graph.
 
         Examples:
             This example converts a model to a graph.
 
             >>> from dwave.optimization.model import Model
             >>> model = Model()
-            >>> c = model.constant(8)
-            >>> i = model.integer((20, 30))
-            >>> g = model.to_networkx()   # doctest: +SKIP
-        """
-        # Todo: adapt to use iter_symbols()
-        # This whole method will need a re-write, it currently only works with gcc
-        # but it is useful for development
+            >>> one = model.constant(1)
+            >>> two = model.constant(2)
+            >>> i = model.integer()
+            >>> model.minimize(two * i - one)
+            >>> G = model.to_networkx()
 
-        import re
-        import networkx
+            One advantage of converting to NetworkX is the wide availability
+            of drawing tools. See NetworkX's
+            `drawing <https://networkx.org/documentation/stable/reference/drawing.html>`_
+            documentation.
 
-        G = networkx.DiGraph()
+            This example uses `DAGVIZ <https://wimyedema.github.io/dagviz/>`_ to
+            draw the NetworkX graph created in the example above.
 
-        cdef cppNode* ptr
-        for i in range(self._graph.num_nodes()):
-            ptr = self._graph.nodes()[i].get()
+            >>> import dagviz                      # doctest: +SKIP
+            >>> r = dagviz.render_svg(G)           # doctest: +SKIP
+            >>> with open("model.svg", "w") as f:  # doctest: +SKIP
+            ...     f.write(r)
 
-            # this regex is compiler specific! Don't do this for the general case
-            match = re.search("\d+([a-zA-z]+Node)", str(typeid(deref(ptr)).name()))
-            if not match:
-                raise ValueError
+            This creates the following image:
 
-            u = (match[1], <long>(ptr))
+            .. figure:: /_images/to_networkx_example.svg
+               :width: 500 px
+               :name: dwave-optimization-to-networkx-example
+               :alt: Image of NetworkX Directed Graph
 
-            G.add_node(u)
+        """
+        import networkx
 
-            for j in range(ptr.predecessors().size()):
-                pptr = ptr.predecessors()[j]
+        G = networkx.MultiDiGraph()
 
-                match = re.search("\d+([a-zA-z]+Node)", str(typeid(deref(pptr)).name()))
-                if not match:
-                    raise ValueError
+        # Add the symbols, in order if we happen to be topologically sorted
+        G.add_nodes_from(repr(symbol) for symbol in self.iter_symbols())
 
-                v = (match[1], <long>(pptr))
+        # Sanity check. If several nodes map to the same symbol repr we'll see
+        # too few nodes in the graph
+        if len(G) != self.num_symbols():
+            raise RuntimeError("symbol repr() is not unique to the underlying node")
 
-                G.add_edge(v, u)
+        # Now add the edges
+        for symbol in self.iter_symbols():
+            for successor in symbol.iter_successors():
+                G.add_edge(repr(symbol), repr(successor))
+
+        # Sanity check again. If the repr of symbols isn't unique to the underlying
+        # node then we'll see too many nodes in the graph here
+        if len(G) != self.num_symbols():
+            raise RuntimeError("symbol repr() is not unique to the underlying node")
+
+        # Add the objective if it's present. We call it "minimize" to be
+        # consistent with the minimize() function
+        if self.objective is not None:
+            G.add_edge(repr(self.objective), "minimize")
+
+        # Likewise if we have constraints, add a special node for them
+        for symbol in self.iter_constraints():
+            G.add_edge(repr(symbol), "constraint(s)")
 
         return G
 
@@ -1193,6 +1212,20 @@ cdef class Symbol:
         # via their subclasses.
         raise ValueError("Symbols cannot be constructed directly")
 
+    def __repr__(self):
+        """Return a representation of the symbol.
+
+        The representation refers to the identity of the underlying node, rather than
+        the identity of the Python symbol.
+        """
+        cls = type(self)
+        # We refer to the node_ptr, which is not necessarily the address of the
+        # C++ node, as it sublasses Node.
+        # But this is unique to each node, and functions as an id rather than
+        # as a pointer, so that's OK.
+        # Otherwise we aim to match Python's default __repr__.
+        return f"<{cls.__module__}.{cls.__qualname__} at {<uintptr_t>self.node_ptr:#x}>"
+
     cdef void initialize_node(self, Model model, cppNode* node_ptr) noexcept:
         self.model = model
 

releasenotes/notes/fix-to_networkx-a78f0f669cc9638c.yaml

@@ -0,0 +1,9 @@
+---
+features:
+  - |
+    Make ``repr()`` of symbols unique to the underlying node, rather than to the Python symbol.
+    See `#52 <https://github.com/dwavesystems/dwave-optimization/issues/52>_.
+fixes:
+  - |
+    Fix ``Symbol.to_networkx()`` to no longer be compiler-dependant.
+    See `#18 <https://github.com/dwavesystems/dwave-optimization/issues/18>_.

tests/test_model.py

@@ -260,6 +260,76 @@ def test_state_size(self):
         model.constant(np.arange(25).reshape(5, 5))
         self.assertEqual(model.state_size(), 25 * 8)
 
+    def test_to_networkx(self):
+        try:
+            import networkx as nx
+        except ImportError:
+            return self.skipTest("NetworkX is not installed")
+
+        model = Model()
+        a = model.binary()
+        b = model.binary()
+        ab = a * b
+
+        G = model.to_networkx()
+
+        self.assertEqual(len(G.nodes), 3)
+        self.assertEqual(len(G.edges), 2)
+
+        # the repr as labels is an implementation detail and subject to change
+        self.assertIn(repr(a), G.nodes)
+        self.assertIn(repr(b), G.nodes)
+        self.assertIn(repr(ab), G.nodes)
+        self.assertIn((repr(a), repr(ab)), G.edges)
+        self.assertIn((repr(b), repr(ab)), G.edges)
+
+        # graph created is deterministic
+        self.assertTrue(nx.utils.graphs_equal(G, model.to_networkx()))
+
+    def test_to_networkx_multigraph(self):
+        try:
+            import networkx as nx
+        except ImportError:
+            return self.skipTest("NetworkX is not installed")
+
+        model = Model()
+        a = model.binary()
+        aa = a * a  # two edges to the same node
+
+        G = model.to_networkx()
+
+        # the repr as labels is an implementation detail and subject to change
+        self.assertEqual(len(G.nodes), 2)
+        self.assertEqual(len(G.edges), 2)
+        self.assertIn(repr(a), G.nodes)
+        self.assertIn(repr(aa), G.nodes)
+        self.assertIn((repr(a), repr(aa)), G.edges)
+
+        # graph created is deterministic
+        self.assertTrue(nx.utils.graphs_equal(G, model.to_networkx()))
+
+    def test_to_networkx_objective_and_constraints(self):
+        try:
+            import networkx as nx
+        except ImportError:
+            return self.skipTest("NetworkX is not installed")
+
+        model = Model()
+        a = model.binary()
+        b = model.binary()
+        model.minimize(a * b)
+
+        G = model.to_networkx()
+        self.assertEqual(len(G.nodes), 4)  # 3 symbols + "minimize"
+        self.assertEqual(len(G.edges), 3)
+        self.assertIn("minimize", G.nodes)
+
+        model.add_constraint(a <= b)
+        G = model.to_networkx()
+        self.assertEqual(len(G.nodes), 6)
+        self.assertEqual(len(G.edges), 6)
+        self.assertIn("constraint(s)", G.nodes)
+
 
 class TestStates(unittest.TestCase):
     def test_clear(self):
@@ -424,3 +494,14 @@ def test_abstract(self):
         from dwave.optimization.model import Symbol
         with self.assertRaisesRegex(ValueError, "Symbols cannot be constructed directly"):
             Symbol()
+
+    def test_repr(self):
+        model = Model()
+        c0 = model.constant(5)
+        c1, = model.iter_symbols()
+        c2 = model.constant(5)
+
+        # the specific form is an implementation detail, but different symbols
+        # representing the same underlying node should have the same repr
+        self.assertEqual(repr(c0), repr(c1))
+        self.assertNotEqual(repr(c0), repr(c2))