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
51 stars 45 forks source link

Make `num_reads` adapt to `initial_states`, if not explicitly given #59

Closed randomir closed 5 years ago

randomir commented 5 years ago

After #53 and #56 are implemented, it would be convenient to have num_reads adapt to the size of the initial_states, IFF initial_states is non-null and num_reads is not provided.

That way it's easier to adaptively sample/anneal off some initial sample set of varying size.

wbernoudy commented 5 years ago

:+1: I think this is expected behavior. For the new parameter that #56 will introduce, if it is set to tile or random, we may want to require num_reads to be provided.

randomir commented 5 years ago

Or, we can just use use a default value for num_reads (currently 10) in that case. That's backward compatible, and also seems nicer.

For example with the default, we could have:

ten_samples = sampler.sample(bqm)

# identical to
ten_samples = sampler.sample(bqm, initial_states_generator='random')

# and
ten_samples = sampler.sample(bqm, initial_states=empty, initial_states_generator='random')

vs

ten_samples = sampler.sample(bqm, num_reads=10)
randomir commented 5 years ago

On second thought, having implicit number of reads (the current behaviour) does seem a bit too magical -- especially if that number is some non-obvious magic constant.

I can see num_reads defaulting to 1, like we have on Tabu.sample, or requiring explicit number from the user...