From c706ae81a39ea7195d3dc1eb683593009b3a1bb5 Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Thu, 11 Jul 2024 09:57:48 -0700 Subject: [PATCH 1/3] Update the default repr() of symbols --- dwave/optimization/model.pyx | 15 +++++++++++++++ .../notes/fix-to_networkx-a78f0f669cc9638c.yaml | 5 +++++ tests/test_model.py | 11 +++++++++++ 3 files changed, 31 insertions(+) create mode 100644 releasenotes/notes/fix-to_networkx-a78f0f669cc9638c.yaml diff --git a/dwave/optimization/model.pyx b/dwave/optimization/model.pyx index 246257f9..629e9897 100644 --- a/dwave/optimization/model.pyx +++ b/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 @@ -1193,6 +1194,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 id of the underlying node, rather than + the id 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 {self.node_ptr:#x}>" + cdef void initialize_node(self, Model model, cppNode* node_ptr) noexcept: self.model = model diff --git a/releasenotes/notes/fix-to_networkx-a78f0f669cc9638c.yaml b/releasenotes/notes/fix-to_networkx-a78f0f669cc9638c.yaml new file mode 100644 index 00000000..63d1e6f7 --- /dev/null +++ b/releasenotes/notes/fix-to_networkx-a78f0f669cc9638c.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Make ``repr()`` of symbols unique to the underlying node, rather than to the Python symbol. + See `#52 _. diff --git a/tests/test_model.py b/tests/test_model.py index 232cdd4c..bca8dde7 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -424,3 +424,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)) From fe1d8010ebf71bfec955657f08f3e53159908e9b Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Thu, 11 Jul 2024 13:24:10 -0700 Subject: [PATCH 2/3] Fix Symbol.to_networkx() --- docs/_images/to_networkx_example.svg | 1 + dwave/optimization/model.pyx | 79 ++++++++++++------- .../fix-to_networkx-a78f0f669cc9638c.yaml | 4 + tests/test_model.py | 70 ++++++++++++++++ 4 files changed, 124 insertions(+), 30 deletions(-) create mode 100644 docs/_images/to_networkx_example.svg diff --git a/docs/_images/to_networkx_example.svg b/docs/_images/to_networkx_example.svg new file mode 100644 index 00000000..a6ca0960 --- /dev/null +++ b/docs/_images/to_networkx_example.svg @@ -0,0 +1 @@ +<dwave.optimization.symbols.Constant at 0x559b56f604a0><dwave.optimization.symbols.Constant at 0x559b568bd0c0><dwave.optimization.symbols.IntegerVariable at 0x559b56d274d0><dwave.optimization.symbols.Multiply at 0x559b57314190><dwave.optimization.symbols.Subtract at 0x559b56ee36f0>minimize \ No newline at end of file diff --git a/dwave/optimization/model.pyx b/dwave/optimization/model.pyx index 629e9897..0e9fc9dc 100644 --- a/dwave/optimization/model.pyx +++ b/dwave/optimization/model.pyx @@ -846,53 +846,72 @@ 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 ` graph. + A :obj:`NetworkX ` 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 `_ + documentation. - G = networkx.DiGraph() + This example uses a `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], (ptr)) + .. figure:: /_images/to_networkx_example.svg + :width: 500 px + :name: 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], (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 + if self.num_constraints(): + for symbol in self.iter_constraints(): + G.add_edge(repr(symbol), "constraint(s)") return G diff --git a/releasenotes/notes/fix-to_networkx-a78f0f669cc9638c.yaml b/releasenotes/notes/fix-to_networkx-a78f0f669cc9638c.yaml index 63d1e6f7..fe0a409e 100644 --- a/releasenotes/notes/fix-to_networkx-a78f0f669cc9638c.yaml +++ b/releasenotes/notes/fix-to_networkx-a78f0f669cc9638c.yaml @@ -3,3 +3,7 @@ features: - | Make ``repr()`` of symbols unique to the underlying node, rather than to the Python symbol. See `#52 _. +fixes: + - | + Fix ``Symbol.to_networkx()`` to no longer be compiler-dependant. + See `#18 _. \ No newline at end of file diff --git a/tests/test_model.py b/tests/test_model.py index bca8dde7..3565ed47 100644 --- a/tests/test_model.py +++ b/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): From db40ff05c021a8a340c2ecc0b3be55455b3f6220 Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Fri, 12 Jul 2024 07:44:56 -0700 Subject: [PATCH 3/3] Tweak Symbol.to_networkx() following review suggestions Co-authored-by: Theodor Isacsson Co-authored-by: Joel Pasvolsky <34041130+JoelPasvolsky@users.noreply.github.com> --- dwave/optimization/model.pyx | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/dwave/optimization/model.pyx b/dwave/optimization/model.pyx index 0e9fc9dc..c6279d17 100644 --- a/dwave/optimization/model.pyx +++ b/dwave/optimization/model.pyx @@ -865,7 +865,7 @@ cdef class Model: `drawing `_ documentation. - This example uses a `DAGVIZ `_ to + This example uses `DAGVIZ `_ to draw the NetworkX graph created in the example above. >>> import dagviz # doctest: +SKIP @@ -877,7 +877,7 @@ cdef class Model: .. figure:: /_images/to_networkx_example.svg :width: 500 px - :name: to-networkx-example + :name: dwave-optimization-to-networkx-example :alt: Image of NetworkX Directed Graph """ @@ -909,9 +909,8 @@ cdef class Model: G.add_edge(repr(self.objective), "minimize") # Likewise if we have constraints, add a special node for them - if self.num_constraints(): - for symbol in self.iter_constraints(): - G.add_edge(repr(symbol), "constraint(s)") + for symbol in self.iter_constraints(): + G.add_edge(repr(symbol), "constraint(s)") return G @@ -1216,8 +1215,8 @@ cdef class Symbol: def __repr__(self): """Return a representation of the symbol. - The representation refers to the id of the underlying node, rather than - the id of the Python 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