Skip to content

Feature/parallel embeddings#569

Merged
randomir merged 13 commits intodwavesystems:masterfrom
jackraymond:feature/parallel_embeddings
Jun 5, 2025
Merged

Feature/parallel embeddings#569
randomir merged 13 commits intodwavesystems:masterfrom
jackraymond:feature/parallel_embeddings

Conversation

@jackraymond
Copy link
Copy Markdown
Contributor

A composite for parallelizing job submissions across structured samplers (i.e. the QPU)

Using minorminer sublattice mappings, this composite should allow us to mark the TilingComposite as deprecated, tiling is a special case of the functionality supported (see examples and tests - all tests passed by TilingComposite are emulated in the new code)

Closes several open issues including tiling support for Zephyr (Adv2) :
#456
#180
#213
It provides an enhanced solution to some other closed cases, like support for advantage: #213

@jackraymond jackraymond reopened this Apr 30, 2025
@jackraymond
Copy link
Copy Markdown
Contributor Author

Tests require this recently integrated bugfix: dwavesystems/minorminer@e40339a, I haven't modified the requirements accordingly.

@jackraymond
Copy link
Copy Markdown
Contributor Author

jackraymond commented Apr 30, 2025

Add Gunnar Millar @gunnarmillar to our team. I'd like him to take a stab at this. Other reviewers welcome. I'd like this functionality to be available for the AQC workshop June 13th (ideally the final practice May 26th).

@jackraymond
Copy link
Copy Markdown
Contributor Author

@JoelPasvolsky This code is already used in the first draft of the AQC workshop repo.

@@ -0,0 +1,520 @@
# Copyright 2018 D-Wave Systems Inc.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2018 D-Wave Systems Inc.
# Copyright 2025 D-Wave

If embeddings and source graph nodes are inconsistent.
If embeddings and target graph nodes are inconsistent.

Examples:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Examples were edited in the wrong place. See the sample() Examples, which will be moved here next commit.

@randomir randomir requested a review from gunnarmiller May 7, 2025 18:28
Copy link
Copy Markdown
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.

The code looks really good. I did a thorough pass over the code and docs. Tomorrow I'll go over the tests.

To fix the CI, you should update minorminer in requirements.txt to 0.2.18 (version with parallel embeddings).

Comment thread dwave/system/composites/parallel_embeddings.py Outdated
Comment thread dwave/system/composites/parallel_embeddings.py Outdated
Comment thread dwave/system/composites/parallel_embeddings.py Outdated
Comment thread dwave/system/composites/parallel_embeddings.py Outdated
Comment thread dwave/system/composites/parallel_embeddings.py Outdated
Comment thread dwave/system/composites/parallel_embeddings.py Outdated
Comment thread dwave/system/composites/parallel_embeddings.py Outdated
Comment thread dwave/system/composites/parallel_embeddings.py Outdated
Comment thread dwave/system/composites/parallel_embeddings.py Outdated
Comment thread dwave/system/composites/parallel_embeddings.py Outdated
Comment thread dwave/system/composites/parallel_embeddings.py Outdated
@randomir randomir force-pushed the feature/parallel_embeddings branch from 6999b06 to d2d2d75 Compare June 4, 2025 19:39
Comment thread tests/test_parallel_embedding_composite.py Outdated
Comment thread tests/test_parallel_embedding_composite.py
Comment thread tests/test_parallel_embedding_composite.py Outdated
@randomir
Copy link
Copy Markdown
Member

randomir commented Jun 4, 2025

@jackraymond, the PR is rebased and cleaned-up.

I reviewed the tests, they LGTM, except the three that are failing (check the CI output). Once you fix those tests, we can merge and release.

@jackraymond
Copy link
Copy Markdown
Contributor Author

@jackraymond, the PR is rebased and cleaned-up.

I reviewed the tests, they LGTM, except the three that are failing (check the CI output). Once you fix those tests, we can merge and release.

The failing tests are because it relies on code in the most recent master of minorminer, but missing from minorminer 0.2.18

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2025

Codecov Report

Attention: Patch coverage is 97.61905% with 2 lines in your changes missing coverage. Please review.

Project coverage is 86.87%. Comparing base (d611fdb) to head (5f581c9).
Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
dwave/system/composites/parallel_embeddings.py 97.59% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #569      +/-   ##
==========================================
- Coverage   88.91%   86.87%   -2.05%     
==========================================
  Files          21       22       +1     
  Lines        1904     1988      +84     
==========================================
+ Hits         1693     1727      +34     
- Misses        211      261      +50     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@randomir randomir merged commit fe634db into dwavesystems:master Jun 5, 2025
20 checks passed
@randomir randomir mentioned this pull request Jun 5, 2025
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.

3 participants