Skip to content

Fix typos and styling. #226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 20, 2023
Merged

Fix typos and styling. #226

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fcbd1a4
Fix typos and styling.
qwriter Feb 3, 2023
1ad8d98
Implement Joel and Alex C's feedback.
qwriter Mar 14, 2023
c164faa
Implement Joel's feedback.
qwriter Mar 15, 2023
373201e
Update dwave_networkx/drawing/chimera_layout.py
qwriter Mar 16, 2023
54a54c0
Remove note about NumPy requirement.
qwriter Mar 16, 2023
f02010d
Merge remote-tracking branch 'origin/copyedit_drawing' into copyedit_…
qwriter Mar 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 30 additions & 25 deletions docs/reference/drawing.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
Drawing
*******

Tools to visualize topologies of D-Wave QPUs and weighted graph problems on them.
Tools to visualize topologies of D-Wave QPUs and weighted :term:`graph` problems on them.

.. currentmodule:: dwave_networkx

.. note:: Some functionality requires `NumPy <https://scipy.org>`_ and/or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"requires NumPy <https://scipy.org>_ and/or" --> both dwave-networkx's dependencies, dimod and networkx, have NumPy as a dependency. Is there any chance dwave-networkx users won't have NumPy? @arcondello

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. We don't have it as a full dependency so theoretically NetworkX and dimod could drop their dependencies on it. That said, making it a full dependency seems reasonable to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be a future enhancement? Do I need to update this sentence?

`Matplotlib <https://matplotlib.org>`_\ .
`Matplotlib <https://matplotlib.org>`_\ .

Chimera Graph Functions
-----------------------
Expand All @@ -20,7 +20,10 @@ Chimera Graph Functions
:toctree: generated/

chimera_layout
chimera_node_placer_2d
draw_chimera
draw_chimera_embedding
draw_chimera_yield

Example
~~~~~~~
Expand Down Expand Up @@ -53,12 +56,12 @@ positions on a Chimera unit cell.
>>> # matplotlib commands to add labels to graphic not shown

.. figure:: ../_images/chimera_layout_0-rightside.png
:align: center
:name: chimera_layout_0-rightside
:scale: 60 %
:alt: Graph H overlaid on a Chimera unit cell.
:align: center
:name: chimera_layout_0-rightside
:scale: 60 %
:alt: Graph H overlaid on a Chimera unit cell.

Graph H (blue) overlaid on a Chimera unit cell (red nodes and black edges).
Graph H (blue) overlaid on a Chimera unit cell (red nodes and black edges).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing :code:`` (or backtic) for the H.
Here's a good place to unobtrusively point out that this Chimera unit cell is rendered in cross layout (because we refer to that in the functions but don't show cross versus column to really clarify that [which we don't do because column drawing is not supported])



Pegasus Graph Functions
Expand All @@ -70,9 +73,10 @@ Pegasus Graph Functions
:toctree: generated/

draw_pegasus
draw_pegasus_embedding
pegasus_layout
pegasus_node_placer_2d
draw_pegasus_embedding
draw_pegasus_yield
pegasus_layout
pegasus_node_placer_2d

Example
~~~~~~~
Expand All @@ -86,22 +90,22 @@ of nodes of a simple 5-node graph on a small Pegasus lattice.
>>> import matplotlib.pyplot as plt
>>> G = dnx.pegasus_graph(2)
>>> H = dnx.pegasus_graph(2, node_list=[4, 40, 41, 42, 43],
edge_list=[(4, 40), (4, 41), (4, 42), (4, 43)])
edge_list=[(4, 40), (4, 41), (4, 42), (4, 43)])
>>> # Show graph H on a small Pegasus lattice
>>> plt.ion()
>>> # Show graph H on a small Pegasus lattice
>>> plt.ion()
>>> dnx.draw_pegasus(G, with_labels=True, crosses=True, node_color="Yellow")
>>> dnx.draw_pegasus(H, crosses=True, node_color='b', style='dashed',
edge_color='b', width=3)
edge_color='b', width=3)

.. figure:: ../_images/pegasus_layout_h_on_g.png
:align: center
:name: pegasus_layout_h_on_g.png
:scale: 60 %
:alt: Graph H overlaid on a Pegasus lattice size 2.
:align: center
:name: pegasus_layout_h_on_g.png
:scale: 60 %
:alt: Graph H overlaid on a Pegasus lattice size 2.

Graph H (blue) overlaid on a small Pegasus lattice(yellow nodes and black edges).
Graph H (blue) overlaid on a small Pegasus lattice(yellow nodes and black edges).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for consistency we should probably :code:`` (backtick) the H

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a good place to also point out that the layout is rendered in cross format to help users understand what "cross" refers to when used elsewhere (be a good result returned when someone searches "cross")


Zephyr Graph Functions
----------------------
Expand All @@ -112,9 +116,10 @@ Zephyr Graph Functions
:toctree: generated/

draw_zephyr
draw_zephyr_embedding
draw_zephyr_yield
zephyr_layout
draw_zephyr_embedding
draw_zephyr_yield
zephyr_layout
zephyr_node_placer_2d

Example
~~~~~~~
Expand All @@ -135,9 +140,9 @@ of a five-node clique on a small Zephyr graph.
>>> dnx.draw_zephyr_embedding(G, embedding, show_labels=True)

.. figure:: ../_images/zephyr_embedding_5clique.png
:align: center
:name: zephyr_embedding_5clique.png
:scale: 60 %
:alt: Five-node clique embedded in a small Zephyr graph.
:align: center
:name: zephyr_embedding_5clique.png
:scale: 60 %
:alt: Five-node clique embedded in a small Zephyr graph.

Five-node clique embedded in a small Zephyr graph.
Five-node clique embedded in a small Zephyr graph.
138 changes: 68 additions & 70 deletions dwave_networkx/drawing/chimera_layout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#
# ================================================================================================
"""
Tools to visualize Chimera lattices and weighted graph problems on them.
Tools to visualize :term:`Chimera` lattices and weighted :term:`graph` problems on them.
"""

import networkx as nx
Expand All @@ -28,33 +28,34 @@


def chimera_layout(G, scale=1., center=None, dim=2):
"""Positions the nodes of graph G in a Chimera cross topology.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrt this comment, could we delete the phrase "in a Chimera cross topology"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function creates a Chimera layout that can then be plotted. It could have potentially realized the layout in different ways, in particular, the cross and column layouts shown here. The modifier "in a Chimera cross topology" is saying that the function selected to implement the cross layout.
"topology" is not the best word choice because it's used for the QPU connectivity (i.e. Chimera structure). So the info is needed but needs improved phrasing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"in a Chimera cross layout"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be okay or perhaps something like, "Positions a graph's nodes in a Chimera layout with unit cells as crosses."
Kelly uses "cross configuration" in Pegasus but I'm not sure that's clear to most users.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the HTML rendering, it seems to me that the modifier "with unit cells as crosses" is given too much importance at the top-level by being in the one-line descriptions. For someone looking for Chimera graph functions, what's important at the top level is that function chimera_layout "Positions the nodes of a graph in a Chimera layout."
If we provided column-layout too, it would make sense to specify at the top level the cross/column info. We just have the one option, so I'm inclined to put that in a separate, next line.

image

"""Positions the nodes of graph ``G`` in a Chimera layout with unit cells as crosses.

NumPy (https://scipy.org) is required for this function.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question about NumPy as in drawing.rst


Parameters
----------
G : NetworkX graph
Should be a Chimera graph or a subgraph of a
Chimera graph. If every node in G has a `chimera_index`
attribute, those are used to place the nodes. Otherwise makes
a best-effort attempt to find positions.
:term:`Chimera` :term:`graph` or :term:`subgraph` of a
Chimera graph. If every node in ``G`` has a ``chimera_index``
attribute, the node position in the ``chimera_index``
attribute is used to place each node. Otherwise,
a best-effort attempt is made to find the node positions.

scale : float (default 1.)
Scale factor. When scale = 1, all positions fit within [0, 1]
Scale factor. If ``scale`` = 1, all positions fit within [0, 1]
on the x-axis and [-1, 0] on the y-axis.

center : None or array (default None)
Coordinates of the top left corner.

dim : int (default 2)
Number of dimensions. When dim > 2, all extra dimensions are
Number of dimensions. If ``dim`` > 2, all extra dimensions are
set to 0.

Returns
-------
pos : dict
A dictionary of positions keyed by node.
Dictionary of positions keyed by node.

Examples
--------
Expand Down Expand Up @@ -106,13 +107,13 @@ def chimera_layout(G, scale=1., center=None, dim=2):


def chimera_node_placer_2d(m, n, t, scale=1., center=None, dim=2):
"""Generates a function that converts Chimera indices to x, y
coordinates for a plot.
"""Generates a function that converts Chimera indices to x- and
y-coordinates for a plot.

Parameters
----------
m : int
Number of rows in the Chimera lattice.
Number of rows in the :term:`Chimera` lattice.

n : int
Number of columns in the Chimera lattice.
Expand All @@ -121,22 +122,21 @@ def chimera_node_placer_2d(m, n, t, scale=1., center=None, dim=2):
Size of the shore within each Chimera tile.

scale : float (default 1.)
Scale factor. When scale = 1, all positions fit within [0, 1]
Scale factor. If ``scale`` = 1, all positions fit within [0, 1]
on the x-axis and [-1, 0] on the y-axis.

center : None or array (default None)
Coordinates of the top left corner.

dim : int (default 2)
Number of dimensions. When dim > 2, all extra dimensions are
Number of dimensions. If ``dim`` > 2, all extra dimensions are
set to 0.

Returns
-------
xy_coords : function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function

A function that maps a Chimera index (i, j, u, k) in an
(m, n, t) Chimera lattice to x,y coordinates such as
used by a plot.
Function that maps a Chimera index ``(i, j, u, k)`` in an
``(m, n, t)`` Chimera lattice to x- and y-coordinates.

"""
import numpy as np
Expand Down Expand Up @@ -190,30 +190,29 @@ def _xy_coords(i, j, u, k):


def draw_chimera(G, **kwargs):
"""Draws graph G in a Chimera cross topology.

If `linear_biases` and/or `quadratic_biases` are provided, these
are visualized on the plot.
"""Draws graph ``G`` in a Chimera layout with unit cells as crosses.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment on "with unit cells as crosses" as in drawing.rst


Parameters
----------
G : NetworkX graph
Should be a Chimera graph or a subgraph of a Chimera graph.
:term:`Chimera` :term:`graph` or a :term:`subgraph` of a Chimera graph.

linear_biases : dict (optional, default {})
A dict of biases associated with each node in G. Should be of
form {node: bias, ...}. Each bias should be numeric.
Linear biases for all nodes of ``G`` as a dict of
the form ``{node: bias, ...}``, where each bias is numeric.
If specified, the linear biases are visualized on the plot.

quadratic_biases : dict (optional, default {})
A dict of biases associated with each edge in G. Should be of
form {edge: bias, ...}. Each bias should be numeric. Self-loop
Quadratic biases for all edges of ``G`` as a dict of
the form ``{edge: bias, ...}``, where each bias is numeric. Self-loop
edges (i.e., :math:`i=j`) are treated as linear biases.
If specified, the quadratic biases are visualized on the plot.

kwargs : optional keywords
See networkx.draw_networkx() for a description of optional keywords,
with the exception of the `pos` parameter which is not used by this
function. If `linear_biases` or `quadratic_biases` are provided,
any provided `node_color` or `edge_color` arguments are ignored.
Parameters in :func:`~networkx.drawing.nx_pylab.draw_networkx`, except for the ``pos`` parameter.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it work with the simpler path networkx.draw_networkx, it will be less fragile to changes in structure than networkx.drawing.nx_pylab.draw_networkx.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. @JoelPasvolsky?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also don't think so, networkx.drawing.nx_pylab.draw_networkx reference/generated/networkx.drawing.nx_pylab.draw_networkx.html#networkx.drawing.nx_pylab.draw_networkx is the only draw_networkx under py:function in the NetworkX objects.inv file that I see

If the ``linear_biases`` or ``quadratic_biases`` parameters are specified,
the :func:`~networkx.drawing.nx_pylab.draw_networkx` ``node_color``
or ``edge_color`` parameters are ignored.

Examples
--------
Expand All @@ -230,87 +229,86 @@ def draw_chimera(G, **kwargs):


def draw_chimera_embedding(G, *args, **kwargs):
"""Draws an embedding onto the chimera graph G, according to layout.

If interaction_edges is not None, then only display the couplers in that
list. If embedded_graph is not None, the only display the couplers between
chains with intended couplings according to embedded_graph.
"""Draws an embedding onto the chimera graph ``G``.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chimera


Parameters
----------
G : NetworkX graph
Should be a Chimera graph or a subgraph of a Chimera graph.
:term:`Chimera` :term:`graph` or a :term:`subgraph` of a Chimera graph.

emb : dict
A dict of chains associated with each node in G. Should be
of the form {node: chain, ...}. Chains should be iterables
of qubit labels (qubits are nodes in G).
Embedding ``emb`` for all nodes of ``G`` as a dict of chains
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Embedding emb for" --> Why repeat the name of the parameter here? "Embedding for ... " works fine

of the form ``{node: chain, ...}``. where chains are iterables
of qubit labels. Qubits are nodes in ``G``.

embedded_graph : NetworkX graph (optional, default None)
A graph which contains all keys of emb as nodes. If specified,
edges of G will be considered interactions if and only if they
exist between two chains of emb if their keys are connected by
an edge in embedded_graph
Graph which contains all keys of the ``emb`` parameter as nodes. If specified,
the edges of ``G`` will be considered interactions if and only if they
exist between two chains of the emb parameter and if their keys are connected by
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"chains of the emb parameter and" --> "chains of the emb parameter and"

an edge in the ``embedded_graph`` parameter; only the couplers between
chains with intended couplings are displayed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"chains with intended couplings are displayed." --> what does "intended couplings" mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure; it is from line 235, which I tried to integrate into the parameter description:

If embedded_graph is not None, the only display the couplers between
chains with intended couplings according to embedded_graph.
Do you have a suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the meaning there to be that only the couplers for edges of G that are considered interactions (i.e., meet the two requirements of the preceding sentence) are displayed.


interaction_edges : list (optional, default None)
A list of edges which will be used as interactions.
Interactions as a list of edges.
If this parameter is specified, only the couplers in the list are displayed.

show_labels: boolean (optional, default False)
If show_labels is True, then each chain in emb is labelled with its key.
If True, each chain in the ``emb`` parameter is labelled with its key.

chain_color : dict (optional, default None)
A dict of colors associated with each key in emb. Should be
of the form {node: rgba_color, ...}. Colors should be length-4
tuples of floats between 0 and 1 inclusive. If chain_color is None,
Chain colors associated with each key in the ``emb`` parameter as a dict
of the form ``{node: rgba_color, ...}``, where colors must be length-4
tuples of floats between 0 and 1, inclusive. If None,
each chain will be assigned a different color.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"each chain will be assigned a different color" --> "each chain is assigned a different color"


unused_color : tuple (optional, default (0.9,0.9,0.9,1.0))
The color to use for nodes and edges of G which are not involved
in chains, and edges which are neither chain edges nor interactions.
If unused_color is None, these nodes and edges will not be shown at all.
Color to use for graph ``G``'s nodes and edges that are not part of
chains, and edges that are neither chain edges nor interactions.
If None, these nodes and edges will not be shown.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"If None, these nodes and edges will not be shown." --> "If None, these nodes and edges are not shown."


overlapped_embedding: boolean (optional, default False)
If overlapped_embedding is True, then chains in emb may overlap (contain
the same vertices in G), and the drawing will display these overlaps as
If True, chains in the ``emb`` parameter may overlap (contain
the same vertices in ``G``), and the drawing will display these overlaps as
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the drawing will display these overlaps" --> "the drawing displays these overlaps"

concentric circles.

kwargs : optional keywords
See networkx.draw_networkx() for a description of optional keywords,
with the exception of the `pos` parameter which is not used by this
function. If `linear_biases` or `quadratic_biases` are provided,
any provided `node_color` or `edge_color` arguments are ignored.
Parameters in :func:`~networkx.drawing.nx_pylab.draw_networkx`, except for the ``pos`` parameter.
If the ``linear_biases`` or ``quadratic_biases`` parameters are specified,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"If the linear_biases or quadratic_biases parameters are specified" --> This function does not have linear_biases or quadratic_biases parameters, at least in the above descriptions.

Copy link
Contributor Author

@qwriter qwriter Mar 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were present in the original. Perhaps they are referring to the parameters in draw_chimera? Should we point to draw_chimera?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest leaving this bit (for all three topographies) alone in this PR because I suspect the current functionality has problems that will be fixed by this PR. Currently, it seems to me that the node_color and edge_color params are not supported:

G = dnx.chimera_graph(2)
embedding = {"N1": [0, 4, 5], "N2": [1, 6, 7]}
dnx.draw_chimera_embedding(G, embedding, show_labels=True, node_color="r")

returns a TypeError: networkx.drawing.nx_pylab.draw() got multiple values for keyword argument 'node_color' error.

In any case the other PR and this change have conflicts so leaving it untouched will save Kelly work.

the :func:`~networkx.drawing.nx_pylab.draw_networkx` ``node_color``
or ``edge_color`` parameters are ignored.
"""
draw_embedding(G, chimera_layout(G), *args, **kwargs)


def draw_chimera_yield(G, **kwargs):
"""Draws the given graph G with highlighted faults, according to layout.
"""Draws the given graph ``G`` with highlighted faults.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Draws the given graph G with highlighted faults" --> consistency: other one-line descriptions don't use "given"


Parameters
----------
G : NetworkX graph
The graph to be parsed for faults
:term:`Graph` to be parsed for faults.

unused_color : tuple or color string (optional, default (0.9,0.9,0.9,1.0))
The color to use for nodes and edges of G which are not faults.
If unused_color is None, these nodes and edges will not be shown at all.
Color to use for graph ``G``'s nodes and edges which are not faults.
If None, these nodes and edges will not be shown.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"these nodes and edges will not be shown" --> "these nodes and edges are not shown"


fault_color : tuple or color string (optional, default (1.0,0.0,0.0,1.0))
A color to represent nodes absent from the graph G. Colors should be
length-4 tuples of floats between 0 and 1 inclusive.
Color to represent nodes that are absent from graph ``G``. Colors must be
length-4 tuples of floats between 0 and 1, inclusive.

fault_shape : string, optional (default='x')
The shape of the fault nodes. Specification is as matplotlib.scatter
marker, one of 'so^>v<dph8'.
Shape of the fault nodes. The shapes are the same as those specified for
`Matplotlib markers <https://matplotlib.org/stable/api/markers_api.html#module-matplotlib.markers>`_.

fault_style : string, optional (default='dashed')
Edge fault line style (solid|dashed|dotted|dashdot)
Line style for fault edges. The line style can be any of the following values:
``'solid'``, ``'dashed'``, ``'dotted'``, ``'dashdot'``.

kwargs : optional keywords
See networkx.draw_networkx() for a description of optional keywords,
with the exception of the `pos` parameter which is not used by this
function. If `linear_biases` or `quadratic_biases` are provided,
any provided `node_color` or `edge_color` arguments are ignored.
Parameters in :class:`.draw_networkx`, except for the ``pos`` parameter.
If the ``linear_biases`` or ``quadratic_biases`` parameters are specified,
the :func:`~networkx.drawing.nx_pylab.draw_networkx` ``node_color``
or ``edge_color`` parameters are ignored.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"If the linear_biases or quadratic_biases parameters are specified" --> This function does not have linear_biases or quadratic_biases parameters, at least in the above descriptions.

"""
try:
assert(G.graph["family"] == "chimera")
Expand Down
Loading