dwavesystems / dwave-neal

An implementation of a simulated annealing sampler for general Ising model graphs in C++ with a dimod Python wrapper.
https://docs.ocean.dwavesys.com/projects/neal/en/latest
Apache License 2.0
50 stars 44 forks source link

Corrected errors in beta_range defaults, simplified discretization pf schedule, added documentation, and beta_schedule parameter #91

Closed jackraymond closed 2 years ago

jackraymond commented 3 years ago

it makes sense to update neal default beta_range correcting inconsistencies and other anomalies:

Performance of time to solution under defaults in a variety of standard models is not significantly changed at QPU relevant scales: 3 regular 3XORSAT, SK model, Viana Bray model, Code Division Multiple Access problems, pegasus structured spin-glasses and cubic lattice spin glasses have been tested. See internal (D-Wave) documentation for further details.

randomir commented 3 years ago

I love this update, @jackraymond! The new API makes much more sense! Few minor comments to follow in the review.

jackraymond commented 3 years ago

I had a discussion with benchmarking (Pau Farre) at D-Wave. The use of the dwave-neal sample() call is so widespread in benchmarking he indicated it would be extremely inconvenient to change the function signature. In this pull request I suggested num_sweeps -> replaced by two arguments (matching underlying C++ code) num_betas and num_sweeps_per_beta. As a compromise, I think a good solution would be just to add the argument num_sweeps_per_beta, along with a requirement that num_sweeps is integer divisible by num_sweeps_per_beta. I think it makes everyone happy, also maintains compatibility with the new 'beta_schedule' parameter.

randomir commented 3 years ago

Hey @pau557, could you please qualify the impact of the new default beta schedule on some standard problems? Ideally, compare it with the old behavior (neal in master). Thanks!

codecov-commenter commented 2 years ago

Codecov Report

Merging #91 (7c539d6) into master (27797d0) will decrease coverage by 14.45%. The diff coverage is 77.01%.

@@             Coverage Diff             @@
##           master      #91       +/-   ##
===========================================
- Coverage   98.66%   84.21%   -14.46%     
===========================================
  Files           3        3               
  Lines          75      133       +58     
===========================================
+ Hits           74      112       +38     
- Misses          1       21       +20     
Impacted Files Coverage Δ
neal/sampler.py 83.06% <77.01%> (-15.43%) :arrow_down:

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 27797d0...7c539d6. Read the comment docs.

necaisej commented 2 years ago

Merge conflict introduced by commit in master updating return type from Response to SampleSet; simple conflict should be resolved @jackraymond