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

Add support for torus QPU graph variations #225

Merged
merged 15 commits into from
Dec 7, 2022

Conversation

jackraymond
Copy link
Contributor

The primary purpose of this pull request is to provide generators for the instances evaluated in https://arxiv.org/abs/2202.03044 [BBRR].

The pull request:

  • Adds support for torus graph variations upon Chimera, Pegasus and Zephyr as used in _[BBRR], _[RH].
  • Corrects errors associated to special edge cases in use of check_edge_list and check_node_list in zephyr_graph, pegasus_graph and chimera_graph
  • Refactors some bibliography entries, and corrects for some associated errors in elimination_order.py
  • Simplifies one line in coloring.py to suppress a type error.

@codecov-commenter
Copy link

codecov-commenter commented Dec 1, 2022

Codecov Report

Merging #225 (ededd31) into main (b0dee61) will increase coverage by 0.47%.
The diff coverage is 95.00%.

@@            Coverage Diff             @@
##             main     #225      +/-   ##
==========================================
+ Coverage   75.18%   75.66%   +0.47%     
==========================================
  Files          31       30       -1     
  Lines        2112     2174      +62     
==========================================
+ Hits         1588     1645      +57     
- Misses        524      529       +5     
Impacted Files Coverage Δ
dwave_networkx/algorithms/elimination_ordering.py 85.57% <ø> (ø)
dwave_networkx/generators/chimera.py 87.16% <93.33%> (+0.46%) ⬆️
dwave_networkx/generators/common.py 95.45% <93.33%> (-4.55%) ⬇️
dwave_networkx/generators/pegasus.py 89.60% <93.75%> (-0.01%) ⬇️
dwave_networkx/algorithms/coloring.py 96.47% <100.00%> (ø)
dwave_networkx/generators/zephyr.py 97.27% <100.00%> (+0.22%) ⬆️
dwave_networkx/__init__.py

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

docs/bibliography.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@JoelPasvolsky JoelPasvolsky left a comment

Choose a reason for hiding this comment

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

I'm making this PR review in slices to help you get it in quickly. This first one is mostly on the chimera.py updates

docs/bibliography.rst Outdated Show resolved Hide resolved
docs/reference/generators.rst Show resolved Hide resolved
dwave_networkx/generators/chimera.py Outdated Show resolved Hide resolved
dwave_networkx/generators/chimera.py Outdated Show resolved Hide resolved
dwave_networkx/generators/chimera.py Outdated Show resolved Hide resolved
dwave_networkx/generators/chimera.py Outdated Show resolved Hide resolved
dwave_networkx/generators/chimera.py Outdated Show resolved Hide resolved
dwave_networkx/generators/chimera.py Outdated Show resolved Hide resolved
dwave_networkx/generators/chimera.py Outdated Show resolved Hide resolved
Copy link
Contributor

@JoelPasvolsky JoelPasvolsky left a comment

Choose a reason for hiding this comment

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

Comments for another slice: pegasus.py

dwave_networkx/generators/pegasus.py Outdated Show resolved Hide resolved
dwave_networkx/generators/pegasus.py Outdated Show resolved Hide resolved
dwave_networkx/generators/pegasus.py Outdated Show resolved Hide resolved
dwave_networkx/generators/pegasus.py Outdated Show resolved Hide resolved
dwave_networkx/generators/pegasus.py Outdated Show resolved Hide resolved
dwave_networkx/generators/pegasus.py Outdated Show resolved Hide resolved
dwave_networkx/generators/pegasus.py Outdated Show resolved Hide resolved
dwave_networkx/generators/pegasus.py Outdated Show resolved Hide resolved
dwave_networkx/generators/pegasus.py Outdated Show resolved Hide resolved
dwave_networkx/generators/pegasus.py Outdated Show resolved Hide resolved
Copy link
Contributor

@JoelPasvolsky JoelPasvolsky left a comment

Choose a reason for hiding this comment

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

Third slice (for Zephyr). I think that should cover most of what I'll spend time looking at

Very nice addition, @jackraymond

dwave_networkx/generators/zephyr.py Outdated Show resolved Hide resolved
dwave_networkx/generators/zephyr.py Outdated Show resolved Hide resolved
dwave_networkx/generators/zephyr.py Outdated Show resolved Hide resolved
dwave_networkx/generators/zephyr.py Outdated Show resolved Hide resolved
dwave_networkx/generators/zephyr.py Outdated Show resolved Hide resolved
dwave_networkx/generators/zephyr.py Outdated Show resolved Hide resolved
dwave_networkx/generators/zephyr.py Outdated Show resolved Hide resolved
dwave_networkx/generators/zephyr.py Outdated Show resolved Hide resolved
dwave_networkx/generators/zephyr.py Outdated Show resolved Hide resolved
dwave_networkx/generators/zephyr.py Outdated Show resolved Hide resolved
Copy link
Contributor

@boothby boothby left a comment

Choose a reason for hiding this comment

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

lgtm

@jackraymond
Copy link
Contributor Author

Implemented review comments in full. Some docstring issues remained in edge_list and node_list, these have been subsequently simplified. Some other minor formatting and compliance issues were addressed.

Copy link
Member

@randomir randomir left a comment

Choose a reason for hiding this comment

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

Tests could use some PEP8 love, but otherwise LGTM!

docs/bibliography.rst Outdated Show resolved Hide resolved
@randomir randomir merged commit feb79f1 into dwavesystems:main Dec 7, 2022
@randomir randomir changed the title Feature/torus Add support for torus QPU graph variations Dec 7, 2022
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

Successfully merging this pull request may close these issues.

None yet

5 participants