-
Notifications
You must be signed in to change notification settings - Fork 57
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.
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
adding graph relabeling and sublattice mappings. #211
Conversation
@pau557 @hhtong @jackraymond I think you'll interact with the sublattice mappings the most; @arcondello @randomir I expect you'll have opinions about that and the graph relabeling interface. |
Codecov Report
@@ Coverage Diff @@
## main #211 +/- ##
==========================================
+ Coverage 71.15% 74.95% +3.79%
==========================================
Files 29 29
Lines 1751 2036 +285
==========================================
+ Hits 1246 1526 +280
- Misses 505 510 +5
Continue to review full report at Codecov.
|
I've been asked to do |
dwave_networkx/generators/pegasus.py
Outdated
if nice_coordinates: | ||
def this_label(): | ||
return pegasus_to_nice(q) | ||
other_name = 'linear_index' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice_index? Also int_label. This code is hard to follow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, yeah, this code is quite annoying. I tried to homogenize the approach behind filling in the labels... I think I can do better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But, no, not nice_index -- other_name
corresponds to the name of the field that's being filled in by other_label
9f8611c
to
eac4b29
Compare
), | ||
) | ||
|
||
def graph_to_pegasus(self, g): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of superficial problems with nice coordinates, might be worth thinking about. How nice do we want nice to be? There are other ways to organize chimera subgraphs without these issues. Having negative coordinate values in the nice scheme is a little bit awkward to me. This could be addressed by a modification of the cell definition (i.e. having a few x, y values of m, rather than a few x,y values of -1 - pushing awkward boundary conditions to high integers rather than near the origin). The nice coordinate scheme indices don't play nice with the visualization tools (draw_zephyr). Variables ordered in x (or y) are not ordered vertically and horizontally in standard visualization. e.g. x=-1 variables are on the interior rather than the boundary.
|
@jackraymond and I had a sidebar; the outcome is to drop the current proposal for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I appreciate the sublattice work
def graph_to_linear(self, g): | ||
"""Return a copy of the graph g relabeled to have linear indices""" | ||
labels = g.graph.get('labels') | ||
if labels == 'int': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have two alternative nomenclatures with linear
and int
coordinates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm following the convention set in chimera_graph
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so int
just gets used inside G.graph["labels"]
for Chimera, Pegasus, Zephyr. Outside of there we call them linear
After time to digest, I came up with a much simpler interface for sublattice mappings. On a D-Wave internal slack channel, this proved to be a popular option. def pegasus_sublattice_mappings(source, target):
"""yields mappings from Chimera or Pegasus graphs into Pegasus graphs"""
def chimera_sublattice_mappings(source, target):
"""yields mappings from Chimera graphs into Chimera graphs"""
def zephyr_sublattice_mappings(source, target):
"""yields mappings from Chimera or Zephyr graphs into Zephyr graphs""" Unable to decide between my original definition of import dwave_networkx as dnx
from matplotlib import pyplot as plt
c2 = dnx.chimera_graph(2)
z2 = dnx.zephyr_graph(2)
for f in dnx.zephyr_sublattice_mappings(c2, z2):
plt.figure(figsize=(10,10))
dnx.draw_zephyr_embedding(z2, {0:[f(v) for v in c2]})
plt.savefig(f"single{''.join(map(str,f.offset))}.png")
plt.close()
c28 = dnx.chimera_graph(2, t=8)
z4 = dnx.zephyr_graph(4)
for f in dnx.zephyr_sublattice_mappings(c28, z4):
plt.figure(figsize=(10,10))
dnx.draw_zephyr_embedding(z4, {0:[f(v) for v in c28]})
plt.savefig(f"double{''.join(map(str,f.offset))}.png")
plt.close() |
e235658
to
e27f7cc
Compare
@arcondello I've rebased & rearranged the deltas to be more cogent |
ffd36c9
to
e27f7cc
Compare
@boothby , can you rebase one more time to fix the CI issues? |
* chimera_coordinates: graph_to_linear graph_to_chimera * pegasus_coordinates: graph_to_linear graph_to_nice graph_to_pegasus * zephyr_coordinates: graph_to_linear graph_to_zephyr
It is often useful to find mappings from small qubit topologies into larger ones. We provide a set of generators, which enable a user to easily adapt an emedding found in a small graph, to one found in a larger graph. This functionality can be used to implement a tiling composite, for example. We provide these functions to find Chimera sublattices in Chimera, Pegasus and Zephyr, as well as Pegasus sublattices in Pegasus and Zephyr sublattices in Zephyr.
e27f7cc
to
f63ff14
Compare
@jackraymond , are you finished reviewing? |
I'm making two significant additions in this PR, and I don't expect it to be merged as-is. Happy to make them separate issues (also todo; documentation is incomplete, there are no tests,
I haven't even checked for obvious typos/omitted imports). Both are additions to thepegasus_coordinates
class. My plan is to repeat this work for Chimera and Zephyr, but I'm pausing here to get some feedback on the interface, and what I'm calling things. Yes please bikeshed thisGraph Converters: I've added three functions to convert graphs from one labeling to another. For example, if you've made a
pegasus_graph
p
with integer labels, but you'd prefer to work with coordinate labels, this saves on boilerplate. What was previouslyis now
Sublattice Mappings: This adds functionality to make homomorphisms from small Pegasus graphs into large Pegasus graphs. This is broken into two steps: (step 1) generating "offsets" which are used to produce (step 2) mappings from a
pegasus_graph(m_small)
topegasus_graph(m_large)
. We show a barely-contrived use case below; searching for fully-yielded P3 subgraphs: