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 a simple sampler-tests generating decorator #627

Merged
merged 7 commits into from
Mar 4, 2020

Conversation

arcondello
Copy link
Member

I also fixed some bugs that this testing framework uncovered. There are two bugs remaining to fix but they are significant enough to be beyond the scope of this PR.

@codecov-io
Copy link

Codecov Report

Merging #627 into master will decrease coverage by 0.11%.
The diff coverage is 95.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #627      +/-   ##
==========================================
- Coverage   90.81%   90.69%   -0.12%     
==========================================
  Files          57       58       +1     
  Lines        3918     4018     +100     
==========================================
+ Hits         3558     3644      +86     
- Misses        360      374      +14
Impacted Files Coverage Δ
dimod/testing/__init__.py 100% <100%> (ø) ⬆️
dimod/traversal.py 100% <100%> (ø) ⬆️
dimod/reference/composites/connectedcomponent.py 97.36% <100%> (+0.14%) ⬆️
dimod/reference/composites/fixedvariable.py 100% <100%> (+3.03%) ⬆️
dimod/core/bqm.py 90.13% <100%> (+0.03%) ⬆️
dimod/reference/composites/spin_transform.py 56.75% <100%> (-38.25%) ⬇️
dimod/testing/sampler.py 94.84% <94.84%> (ø)
dimod/roof_duality/fix_variables.py 90.9% <0%> (+18.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dbcce3f...6d1285b. Read the comment docs.

@arcondello arcondello requested a review from m3ller March 4, 2020 18:42
Copy link
Member

@wbernoudy wbernoudy left a comment

Choose a reason for hiding this comment

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

Great stuff, this will catch many bugs!

Some possible future improvements:

  • Allow solver developers to pass in "extra" BQMs to be tested. Maybe for each BQM, convert to each BQM class and vartype.
  • We could consider a optional tests that try to ensure the samples returned actually line up with the variable labelling, e.g. pass in a problem with just large hs and check to make sure the samples line up. Obviously this is tricky with heuristics.

@arcondello
Copy link
Member Author

This covers the labelling and the large problems can be filtered using the max_num_variables parameter. Or by line up do you mean correct answers?

dimod/testing/sampler.py Show resolved Hide resolved
tests/test_scalecomposite.py Show resolved Hide resolved
@arcondello arcondello merged commit b780a4a into dwavesystems:master Mar 4, 2020
@arcondello arcondello deleted the sampler-tests branch March 4, 2020 21:45
@wbernoudy
Copy link
Member

I mean "correct" answers. If I submit h={0: -100, 1: 100}, and get magnetizations of {0: -0.98, 1: 0.99}, the labeling is probably reversed.

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

Successfully merging this pull request may close these issues.

4 participants