Add Batched requests to Google's cirq messages #3064
Conversation
- Adds BatchedProgram, BatchedRunContext, and BatchedResult - Each is a repeated message so that circuits can be bundled together as one request for increased efficiency.
cirq/google/api/v2/result.proto
Outdated
|
|
||
| // The results of a BatchedProgram | ||
| // Each result in the message will directly correspond to a Program | ||
| // in the BatchedProgram request. There will be one Result for each |
There was a problem hiding this comment.
I think this should be slightly reworded, since there is a result per Program/RunContext pair. I think it is important in the protos to convey the state of the resources, users may thinking about this as "run a program", but we should make it clear in the api that just creating a batched program does not ended up with results.
There was a problem hiding this comment.
Reworded. Take another look and let me know if it is more clear. I wasn't sure if I understood your concerns correctly.
cirq/google/api/v2/program.proto
Outdated
| } | ||
|
|
||
| // A Batch of multiple circuits that should be run as one program | ||
| message BatchedProgram { |
There was a problem hiding this comment.
Nit: call this BatchProgram instead? Similarly for BatchRunContext and BatchResult.
cirq/google/api/v2/program.proto
Outdated
| // A Batch of multiple circuits that should be run as one program | ||
| message BatchedProgram { | ||
|
|
||
| // The circuits that should be bundled togather as one program |
cirq/google/api/v2/program.proto
Outdated
| } | ||
| } | ||
|
|
||
| // A Batch of multiple circuits that should be run as one program |
There was a problem hiding this comment.
Might want to clarify here that "as one program" is at the level of the quantum engine api. These will not necessarily be executed all at once on the hardware.
cirq/google/api/v2/result.proto
Outdated
| repeated SweepResult sweep_results = 1; | ||
| } | ||
|
|
||
| // The result returned by a BatchedProgram |
There was a problem hiding this comment.
nit: (related to @dabacon's comment below)
"The result returned from running a BatchProgram/BatchRunContext"
cirq/google/api/v2/run_context.proto
Outdated
|
|
||
| // A batch of contexts for running a bundled batch of programs | ||
| // To be used in conjunction with BatchedProgram | ||
| message BatchedRunContext { |
There was a problem hiding this comment.
You might consider moving all these into a new batch.proto file, so they are closer together and the relationship between them should be easier to see:
# batch.proto
message BatchProgram { ... }
message BatchRunContext { ... }
message BatchResult { ... }
maffoo
left a comment
maffoo
left a comment
There was a problem hiding this comment.
LGTM
(not sure why there's a diff in old generated proto files)
| message BatchRunContext { | ||
|
|
||
| // Run contexts for each program in the BatchProgram | ||
| // Each RunContext should map directly to a Program in the corresponding |
There was a problem hiding this comment.
Nit: maybe add lines between paragraphs as you've done for BatchResult:
// Run contexts for ...
//
// Each RunContext should map ...
cirq/google/api/v2/batch.proto
Outdated
| // A Batch of multiple circuits that should be run together | ||
| // as one QuantumProgram within Quantum Engine. | ||
| // (Note that circuits may not necessarily be executed | ||
| // simultaneously on hardware based on circumstances) |
There was a problem hiding this comment.
nit: "executed as a batch" instead of "executed simultaneously"
There was a problem hiding this comment.
I think the wording you proposed adds more confusion. I rephrased again.
| // Results returned from executing a BatchProgram/BatchRunContext pair. | ||
| // | ||
| // After a BatchProgram and BatchRunContext is successfully run in | ||
| // Quantum Engine, the expected result if successful will be a BatchResult. |
There was a problem hiding this comment.
Must success be for every program/context combo, or can this include partial results?
There was a problem hiding this comment.
I suspect we would not initially support partial result, but put in a paragraph on how it would be supported in the future.
cirq/google/api/v2/batch.proto
Outdated
| // Each RunContext should map directly to a Program in the corresponding | ||
| // BatchProgram. | ||
| // | ||
| // This message should have one RunContext for each Program in the |
There was a problem hiding this comment.
"should" should be "must" when describing the relationship between run contexts and programs.
- Add Batched requests to Google's cirq messages - This adds a new batch.proto which contains BatchedProgram, BatchedRunContext, and BatchedResult - Each is a repeated message so that circuits can be bundled together as one request for increased efficiency.
5 participants
together as one request for increased efficiency.