Skip to content
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

edge_list and node_list behave unexpectedly in graph generators #245

Open
jackraymond opened this issue Nov 6, 2024 · 5 comments
Open

Comments

@jackraymond
Copy link
Contributor

jackraymond commented Nov 6, 2024

Description
It is useful to permute node and edge orderings in graph creation for example to randomize the behaviour of otherwise deterministic methods.

  • dwave_networkx graph node ordering doesn't seem to respond to the ordering of the node_list parameter.
  • When the edge_list parameter is provided matching the default, the node_ordering is changed relative to the default.

To Reproduce

T = dnx.pegasus_graph(2)
node_list = list(T.nodes())
node_list.reverse()
T2 = dnx.pegasus_graph(2, node_list=node_list)  # Same graph
print(T2.nodes())  # original ordering, ignores provided ordering.
edge_list = list(T1.edges())
node_list = list(T1.nodes())
T3 = dnx.pegasus_graph(2, node_list=node_list, edge_list=edge_list)
print(T3.nodes())  # Does not match T1.nodes(), which is unexpected.

Expected behavior
node_list should dictate node order, edge_list should dictate edge order.

Environment:

  • OS: [Ubuntu 22.04.3 LTS]
  • Python version: [e.g. 3.12.0]

Additional context
I'll make a pull request to correct this unless a good case can be made not to, don't really want to mess with these core generators unnecessarily.
I want to use in combination with minorminer.subgraph.find_subgraph(S, T) which deterministically searches for a subgraph. Reordering the target (T) nodes allows a more uniform distribution across the processor with respect to some source graph S.

@arcondello
Copy link
Member

arcondello commented Nov 6, 2024

My concern is much more with NetworkX than with our generators. I see that in 3.0 Networkx dropped OrderedGraph because the underlying Python dictionaries are now ordered. So at least in principal there is some claim of ordering provided. But I see very little documented guaranteeing that that order is maintained by networkx functions/methods. So my expectation is that code relying on the order may be fragile.

It feels like if we want this in order to get better results from minorminer.subgraph, we should make the change over these.

@jackraymond
Copy link
Contributor Author

jackraymond commented Nov 6, 2024

For my use case I thought of a workaround, since subgraph doesn't require any of the baggage of the dwave_networkx
class I'll just create a new graph with networkx where provided edge and node lists are treated as ordered. So not in a hurry to fix this afterall

@arcondello
Copy link
Member

It also appears that minorminer.subgraph traverses only the edgelist for what it's worth, see https://github.com/dwavesystems/minorminer/blob/6d7df5780898061dd6eb3d39bcee0cb487c2f9ab/minorminer/subgraph.pyx#L307-L319

@jackraymond
Copy link
Contributor Author

Good to know, will just randomize edgelist for this use case.

@jackraymond
Copy link
Contributor Author

Re: only running over the edge set in minorminer.subgraph sounds problematic to me:
dwavesystems/minorminer#254

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants