feat: Introduce MultiSimulator class #260
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #260 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 48 48
Lines 3712 3712
Branches 889 890 +1
=========================================
Hits 3712 3712 ☔ View full report in Codecov by Sentry. |
| @@ -63,3 +63,33 @@ def run( | |||
| @abstractmethod | |||
| def properties(self) -> DeviceCapabilities: | |||
| """DeviceCapabilities: Properties of the device.""" | |||
|
|
|||
|
|
|||
| class MultiSimulator(BraketSimulator): | |||
There was a problem hiding this comment.
It seems like a little overkill to create a new abstract class/interface this one method that's very related to the existing run method.
There was a problem hiding this comment.
Yes, I'm also curious why we couldn't just add run_multiple to the existing interface, and provide a default implementation.
There was a problem hiding this comment.
Regardless, we're going to need a new method for the backend to run multiple circuits; this means it has to live either in BraketSimulator or a new class. If it lives in BraketSimulator, then the SDK would need either check whether the method throws an exception (not great), or we'd need to add a new property in BraketSimulator for the SDK to check, which then leads to a situation where devs can implement the method and forget to mark the property true, or vice-versa.
There was a problem hiding this comment.
Could BraketSimulator also provide a default implementation of run_multiple? (maybe it could do exactly what the SDK does today for run_batch - delegate to Pool)
There was a problem hiding this comment.
I suppose it could; it just doesn't seem useful to have the same implementation in both the simulator and the SDK; as well, the abstract BraketSimulator class having a default implementation could potentially lead to multiple inheritance issues down the line.
There was a problem hiding this comment.
Alternative implementation in #264
| """ | ||
|
|
||
| @abstractmethod | ||
| def run_multiple( |
There was a problem hiding this comment.
We should probably have a default implementation of this, so that future simulators don't need to re-implement it.
There was a problem hiding this comment.
Why do we call it run_multiple instead of run_batch? It's also a little confusing because it's unspecified how the *args and **kwargs should apply to the batch. For example, do they apply globally to all of the tasks? Or should these be lists of the (*args, **kwargs) that would be provided to a single run call?
There was a problem hiding this comment.
The SDK already provides the default implementation if the simulator doesn't subclass MultiSimulator; if someone doesn't want a native run_multiple method, they can just implement BraketSimulator.
As for run_multiple vs run_batch, the reasons are twofold:
- The simulator doesn't really have any notion of "batch" the way the SDK does; the method just returns a list of results.
- The word "batch" is already used in the state vector simulator for subdividing the circuit into chunks of gates and contracting these "batches" together (I'm sorry about the name, my bad), so I didn't want to reuse it.
| Extra arguments will contain any additional information necessary to run the tasks, | ||
| such as number of qubits. |
There was a problem hiding this comment.
I know this is copied from the run docstring, but "number of qubits" is a weird example here. Better examples would be number of shots, or lists of input parameters to the tasks.
|
@speller26 should we close this now in favor of #264? |
Issue #, if available:
Description of changes:
Introduces MultiSimulator class to allow backends to leverage their own batching implementations. Will next publish SDK PR to make use of this interface.
Testing done:
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.