dwavesystems / dwave-system

An API for easily incorporating the D-Wave system as a sampler, either directly or through Leap's cloud-based hybrid samplers
https://docs.ocean.dwavesys.com/
Apache License 2.0
87 stars 62 forks source link

Add ``estimate_qpu_access_time`` method #461

Closed JoelPasvolsky closed 2 years ago

JoelPasvolsky commented 2 years ago

I still need to unit tests next week but would like @gmarsden to take a look at the formulas.

JoelPasvolsky commented 2 years ago
  • accepting same parameters as .sample() methods would be nice (inferring number of qubits from problem submitted, etc)

Do you mean providing the estimate when someone submits a problem (we want to estimate without submitting) or learning the number of qubits from a BQM (will vary because is heuristic) or something else?

Let's talk about cloud-client versus DWaveSampler

Edit: on my way to the gym it occurred to me that you might mean that the function should inherit dimod sample mixins and then if the user doesn't provide an embedding/number of qubits this method could derive a number of qubits from the binary quadratic model, and the user can just copy the parameters from the sample() they plan to do to this function. That would work in dwave-system but for cloud-client we'd need to import minorminer.

gmarsden commented 2 years ago

Do you mean providing the estimate when someone submits a problem (we want to estimate without submitting) or learning the number of qubits from a BQM (will vary because is heuristic) or something else?

I read this as having estimate_qpu_access_time take the same parameters as sample so that the user doesn't have to do work to translate to the parameters (doesn't need to count qubits, set flags, etc) -- just passes the same params.

JoelPasvolsky commented 2 years ago

@randomir, I addressed your comment's 1st bullet by replacing Boolean reverse_anneal with a check on actual param anneal_schedule for reverse annealing, so now the method can accept the same params as the sample() method. I did not yet default to having the method estimate the number of embedded qubits because that can be slow and if we move this to cloud-client we'd need to add minorminer there too and also because whether or not we should do this depends on the user's workflow

randomir commented 2 years ago

Thanks, @JoelPasvolsky. Having a low-level estimator like this -- only encoding the (versioned) rule, without any smarts like embedding -- is a great start. I wasn't trying to suggest we should embed a logical problem here just for time estimation's sake. That would an absolute overkill (given embedding's complexity).

However, I still think this function (in its current form) is better suited in the cloud-client (for reusability), but I could be convinced otherwise.

JoelPasvolsky commented 2 years ago

Ported to dwave-cloud-client