Support all structured samplers in SubproblemCliqueEmbedder #271
Conversation
Codecov Report
@@ Coverage Diff @@
## master #271 +/- ##
==========================================
+ Coverage 92.46% 92.53% +0.06%
==========================================
Files 17 17
Lines 2071 2063 -8
==========================================
- Hits 1915 1909 -6
+ Misses 156 154 -2
Continue to review full report at Codecov.
|
tests/test_samplers.py
Outdated
| @@ -109,12 +112,12 @@ def test_external_embedding_sampler(self): | |||
| # verify mock sampler received custom kwargs | |||
| self.assertEqual(res.subsamples.first.energy, -1) | |||
|
|
|||
| def test_clique_embedder(self): | |||
| @parameterized.expand([['chimera', 2], ['pegasus', 1]]) | |||
There was a problem hiding this comment.
Can you add a test for Zephyr as well?
There was a problem hiding this comment.
Now I see your comment above. You can up the lower bound in tests/requirements.txt only.
| @@ -1,3 +1,4 @@ | |||
| coverage | |||
| codecov | |||
| parameterized | |||
| dwave-system>=1.13.0 | |||
There was a problem hiding this comment.
I see this creates a conflict with main requirements file. We'll have to update it there instead.
There was a problem hiding this comment.
When I tested locally, I ran pip install -r requirements.txt and pip install -r test/requirements.txt sequentially. And that worked. I see that CircleCI (.circleci/config.yml) is doing pip install -r requirements.txt -r test/requirements.txt in one command. So, alternatively, I suppose we could change .circleci/config.yml to run those commands sequentially. Should I try that?
Fixes --- - Support all structured samplers in SubproblemCliqueEmbedder (dwavesystems#271)
Fixes #264
To be clear, all structured samplers with topologies that are supported by
minorminer.buscliqueare supported inSubproblemCliqueEmbedderwith this change.Both chimera- and pegasus-structured (mock) samplers are explicitly tested in composition with
SubproblemCliqueEmbedderin unit tests with this change. Support for zephyr-structured samplers has been confirmed with manual testing.Not adding a unit test for Zephyr at this time - we will have to changerequirements.txtto depend on a newer [1]dwave-systemfor that.[1] Not sure what is the exact minimum version that we would need for this, but it is no less than1.13.0as we need dwavesystems/dwave-system@79fcf55Edit: Added a unit test for Zephyr also.