Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Introduce MultiSimulator class #260

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/braket/default_simulator/simulator.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import uuid
import warnings
from abc import ABC, abstractmethod
from abc import abstractmethod
from collections.abc import Callable
from typing import Any, Union

Expand Down Expand Up @@ -61,7 +61,7 @@
)


class OpenQASMSimulator(BraketSimulator, ABC):
class OpenQASMSimulator(BraketSimulator):
"""An abstract simulator that runs an OpenQASM 3 program.

Translation of individual operations and observables from OpenQASM to the desired format
Expand Down
2 changes: 1 addition & 1 deletion src/braket/simulator/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@
# ANY KIND, either express or implied. See the License for the specific
# language governing permissions and limitations under the License.

from braket.simulator.braket_simulator import BraketSimulator # noqa: F401
from braket.simulator.braket_simulator import BraketSimulator, MultiSimulator # noqa: F401
30 changes: 30 additions & 0 deletions src/braket/simulator/braket_simulator.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,33 @@ def run(
@abstractmethod
def properties(self) -> DeviceCapabilities:
"""DeviceCapabilities: Properties of the device."""


class MultiSimulator(BraketSimulator):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm also curious why we couldn't just add run_multiple to the existing interface, and provide a default implementation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative implementation in #264

"""An abstract simulator that can run multiple tasks locally.

The purpose of this interface is to take advantage of any circuit batching (or more generally,
multithreading/multiprocessing) that the backend can leverage to improve performance.

The tasks can be either quantum circuits defined as OpenQASM or JAQCD programs,
or analog Hamiltonian simulation (AHS) programs.
"""

@abstractmethod
def run_multiple(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably have a default implementation of this, so that future simulators don't need to re-implement it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. The simulator doesn't really have any notion of "batch" the way the SDK does; the method just returns a list of results.
  2. 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.

self, payloads: list[Union[OQ3Program, AHSProgram, JaqcdProgram]], *args, **kwargs
) -> list[Union[GateModelTaskResult, AnalogHamiltonianSimulationTaskResult]]:
"""
Run the tasks specified by the given IR payloads.

Extra arguments will contain any additional information necessary to run the tasks,
such as number of qubits.
Comment on lines +85 to +86
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


Args:
payloads (list[Union[OQ3Program, AHSProgram, JaqcdProgram]]): The IR representations
of the programs

Returns:
list[Union[GateModelTaskResult, AnalogHamiltonianSimulationTaskResult]]: A list of
result objects, with the ith object being the result of the ith program.
"""