Add run_batch to Sampler and QuantumEngineSampler #3265

kevinsung · 2020-08-25T00:53:29Z

Resolves #3224 .

zaqqwerty · 2020-08-25T03:15:09Z

cirq/work/sampler.py

+            params_list: Parameter sweeps to use with the circuits. The number
+                of sweeps should match the number of circuits and will be
+                paired in order with the circuits.
+            repetitions: Number of circuit repetitions to run. Each sweep value


Quick question here, was wondering what the reasoning is around keeping the number of repetitions fixed across all circuits, rather than able to be specified per-circuit? I imagine there are situations where a user might want to get a different number samples from each circuit. For example, in equation 60 of this paper there is an expected number of samples required to achieve a desired precision that is different for each circuit.

Perhaps @dstrain115 or @maffoo can answer that question. Here I'm just following the interface of the already existing method cirq.google.Engine.run_batch.

Currently, you do not receive any speed up if you specify different numbers of repetitions per circuit since the hardware cannot batch them together. This is specific to quantum engine, which makes me think that maybe we should not add this to Sampler.

@dstrain115, I agree that the batching is very specific currently to Quantum Engine, and it seems that it might be too early to create this abstraction as we simply don't know how/whether other NISQ devices will utilize batching (we should probably ask the Pasqal/AQT/IQM folks!).

On the other hand I see two drivers for this to happen:

simulator/hardware gap and hardware/hardware gap - switching from simulation to hardware now has a friction point - at the extreme the user will have to maintain 5 different circuit execution strategies for simulation, Pasqal, AQT, IQM, Google Quantum Engine...it would be great to increase the overlap and reduce the amount of special code required for each architecture

A similar question comes up in case of downstream users - e.g. how will TFQ leverage batching in QE? It seems that the Sampler interface is the main entrypoint, it would simplify maintenance for them. If we keep run_batch on the QE level, then they'll have to implement logic that goes below the Sampler abstraction level.

@kevinsung what is your main use case for this?

I was planning to use this to speed up XEB benchmarking: https://github.com/quantumlib/Cirq/blob/master/cirq/experiments/grid_parallel_two_qubit_xeb.py.
As is typical of the code in the experiments/ directory, that code works at the Sampler abstraction level.

Sending batches of circuits is also useful for simulators: knowing all circuits upfront, rather than seeing them one at a time, allows various optimizations to be made. For example the batch of circuits can be parallelized: in TFQ, in order to use the cirq density matrix simulator (which inherits from cirq.Sampler), we use the code here to parallelize over batches of circuits. If there were a run_batch call in the API of cirq.Sampler, then this parallelization logic could be moved into the implementation of the simulator.

We also use batching functionality for cirq-backend wavefunction simulations (for things that inherit cirq.SimulatesFinalState), and a similar logic would lead to the creation of a new API call simulate_batch.

So in our use case an answer to the question "What is batching for?" is to take advantage of multi-threaded machines to speed up simulation, and I think the reason to do it in the simulator itself is to reduce code duplication in the case that users besides us look to take advantage of this parallelization across circuits.

I wanted to clear up confusion around the phrase "it could be interpreted as a sweep over repetitions": I think it is not quite a sweep over repetitions, rather I meant that each circuit in the batch would be paired with a number of samples to extract from it; so that len(repetitions) == len(programs). The API could also accept a single number, in which case it pads out repetitions to contain len(programs) copies of the passed number.

Sorry for the several comments, just want to be sure our use case is understood; if it still seems too specific to merit the API change I understand.

Thanks for your comments and providing the TFQ/simulator perspective @zaqqwerty and sorry for the confusion about repetitions.

balopat

I would recommend closing this. See my previous comment. Sorry for not thinking this through upfront on the issue in the first place.

mpharrigan · 2020-08-31T21:27:43Z

how do you plan to support a development flow where you can test code on a simulator and then jump to "the real thing" while using batching?

balopat · 2020-09-01T01:16:24Z

how do you plan to support a development flow where you can test code on a simulator and then jump to "the real thing" while using batching?

Great question.

My main hypothesis is that in the NISQ era we will not be able to invent/enforce a perfect abstraction that will work across all simulators and all quantum platforms that also allows for optimal execution on all these backends.

To your question: anywhere outside the boundaries of "common functionality" the user will have to drop down to backend specific logic to leverage the specific features of that backend. I.e I might be completely off track here but something like:

def get_samples(device) -> cirq.TrialResult: 
    circuits = create_circuits_sweeps()
    if device == cirq.google.Engine(): 
       batches = _prepare_batches(circuits)
       device.run_batch(batches)
    else: 
       device.run_sweep(circuits)

i.e. the user code will have to handle explicitly the cases of different platforms.

Now, the Google code could offer a simulated version - to try out. That might be a good idea, so that you can test the batching logic in your code without having to run it against the real service.

The question is where to draw that boundary and how. I think the boundary is around "unoptimized, generic workflows and/or universally applicable optimizations". If we can do optimization in a generic way, we should include that in our abstract classes. If we cannot, we should keep it in the subclasses.

If we see that a certain optimization appears in most of our backends (e.g. batching turns out to be exactly the same for all our platforms) then it's probably worthwhile to pull it up the stack. But before we have this confidence it will just create confusion.

maffoo · 2020-09-01T15:33:26Z

I think specifying a batch of circuits that can't be expressed as a sweep is a pretty natural thing to want to do in many NISQ experiments, so I would suggest that we should include support for this in the standard cirq.Sampler interface. This seems more user-friendly than requiring users to special-case their code depending on what backend they are running against. As in this PR, there's a natural default implementation of run_batch that just loops over the batch and runs the circuits individually, so there's no extra implementation overhead for sampler backends that don't natively support batching.

An alternative to this kind of "explicit batching" is to have the sampler implementation implicitly batch things and exeecute them in an optimal way on the hardware. This is what we do internally, but it requires being able to submit multiple run calls without waiting which means we need to work out the model for async execution, which is not trivial. Explicit batching is something we can add now without all the async complications.

kevinsung · 2020-09-01T15:48:54Z

Yes, as @maffoo said, there is no extra cost to using run_batch if the backend doesn't natively support it. So the answer to the question "What is batching for?" is simply that it may lead to faster executions on some backends. I don't think that is a confusing answer. The analogous answer could be given to the question "What is async for?" to justify the run_async method of Sampler.

kevinsung · 2020-09-01T16:01:52Z

@maffoo 's comment suggests another answer to the question "What is batching for?" which is that run_batch is actually just a natural operation to perform in many situations. It can be used simply to make code nicer. Note that this suggests we should add it to the Sampler interface even if it is never supported by any hardware.

balopat · 2020-09-01T18:06:53Z

Thank you for all the comments - I'm almost convinced to introduce this. The final question is still around the seeming difference in the backends: the repetitions. Is it a single number or one for each circuit? Would TFQ batching benefit from having a single number or is it an unnecessary limitation for the user? I would go with the more generic case in the interface and then the user can discover that in order to get any speed advantage on different platforms they'll have to parametrize batching accordingly, which in QE case will be the same repetition for each circuit.

kevinsung · 2020-09-02T10:53:52Z

I would go with the more generic case in the interface and then the user can discover that in order to get any speed advantage on different platforms they'll have to parametrize batching accordingly, which in QE case will be the same repetition for each circuit.

I agree.

zaqqwerty · 2020-09-02T14:27:59Z

I also agree about going with the more generic case in the interface, since for the TFQ use case at least it will be most useful if the repetitions are able to be specified per-circuit in the batch.

balopat

Updated review: based on the discussion let's make this work in the Sampler interface, but let's change the repetitions to be per program. We should also add ample explanation in the docstring to encourage users to check the docs on the actual device / simulator they are using to figure out what parametrization will lead to actual speed up.

kevinsung · 2020-09-03T16:04:31Z

Cool. It would be great if I can also address #3285 here. The proposal is that the user should not be forced to pass in a list of parameter resolvers; it should just work if no resolvers are passed.

kevinsung · 2020-09-03T19:42:39Z

I have updated this PR to allow the number of repetitions to be a list and documented that child classes may have requirements that must be met to obtain a speedup. I have also updated the behavior of the method so that it works if no sweeps are provided. However, I didn't modify the behavior of Engine.run_batch, so this does not fix #3285 .

balopat · 2020-09-03T19:45:35Z

I have updated this PR to allow the number of repetitions to be a list and documented that child classes may have requirements that must be met to obtain a speedup. I have also updated the behavior of the method so that it works if no sweeps are provided. However, I didn't modify the behavior of Engine.run_batch, so this does not fix #3285 .

Sounds good - I forgot to reply: let's do that in a separate PR to keep things clean / small.

balopat · 2020-09-03T19:47:45Z

This looks good to me. @dstrain115 can you please have a look too at the final result?

balopat

LGTM

cirq/google/engine/engine_sampler.py

cirq/google/engine/engine_sampler_test.py

cirq/work/sampler.py

kevinsung · 2020-09-11T16:41:34Z

@dstrain115 I've addressed your comments; could you take another look?

dstrain115

LGTM with a few small nits.

cirq/google/engine/engine_sampler.py

cirq/work/sampler.py

kevinsung added 3 commits August 24, 2020 17:51

add run_batch to Sampler

193ac6e

add run_batch to QuantumEngineSampler

5f39284

format

e877b3e

kevinsung requested a review from dstrain115 August 25, 2020 00:53

googlebot added the cla: yes Makes googlebot stop complaining. label Aug 25, 2020

zaqqwerty reviewed Aug 25, 2020

View reviewed changes

balopat suggested changes Aug 31, 2020

View reviewed changes

balopat suggested changes Sep 2, 2020

View reviewed changes

kevinsung added 2 commits September 3, 2020 15:24

allow variable number of repetitions and make params_list optional

3a50ec3

add tests

4db6642

balopat approved these changes Sep 3, 2020

View reviewed changes

dstrain115 reviewed Sep 4, 2020

View reviewed changes

cirq/google/engine/engine_sampler.py Outdated Show resolved Hide resolved

cirq/google/engine/engine_sampler_test.py Show resolved Hide resolved

cirq/work/sampler.py Outdated Show resolved Hide resolved

kevinsung and others added 2 commits September 10, 2020 15:12

Merge branch 'master' into run_batch

27f2864

return list of lists

eff62ed

dstrain115 approved these changes Sep 11, 2020

View reviewed changes

cirq/google/engine/engine_sampler.py Outdated Show resolved Hide resolved

cirq/google/engine/engine_sampler.py Outdated Show resolved Hide resolved

cirq/work/sampler.py Outdated Show resolved Hide resolved

kevinsung added 2 commits September 11, 2020 14:43

update Engine batching requirements and improve error messages

d80a5e7

comma

909fec3

kevinsung added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Sep 11, 2020

CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Sep 11, 2020

Merge branch 'master' into run_batch

30c46e1

CirqBot merged commit a98808a into quantumlib:master Sep 11, 2020

CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Sep 11, 2020

kevinsung deleted the run_batch branch September 16, 2020 14:48

mpharrigan mentioned this pull request May 16, 2022

Add cg.ProcessorSampler #5361

Merged

Add run_batch to Sampler and QuantumEngineSampler #3265

Add run_batch to Sampler and QuantumEngineSampler #3265

Uh oh!

Conversation

kevinsung commented Aug 25, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zaqqwerty Aug 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zaqqwerty Aug 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balopat left a comment

Choose a reason for hiding this comment

Uh oh!

mpharrigan commented Aug 31, 2020

Uh oh!

balopat commented Sep 1, 2020

Uh oh!

maffoo commented Sep 1, 2020

Uh oh!

kevinsung commented Sep 1, 2020

Uh oh!

kevinsung commented Sep 1, 2020

Uh oh!

balopat commented Sep 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinsung commented Sep 2, 2020

Uh oh!

zaqqwerty commented Sep 2, 2020

Uh oh!

balopat left a comment

Choose a reason for hiding this comment

Uh oh!

kevinsung commented Sep 3, 2020

Uh oh!

kevinsung commented Sep 3, 2020

Uh oh!

balopat commented Sep 3, 2020

Uh oh!

balopat commented Sep 3, 2020

Uh oh!

balopat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevinsung commented Sep 11, 2020

Uh oh!

dstrain115 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

zaqqwerty Aug 31, 2020 •

edited

Loading

zaqqwerty Aug 31, 2020 •

edited

Loading

balopat commented Sep 1, 2020 •

edited

Loading