Add asynchronous ability to simulated engine. #4811
Conversation
Adds ASYNCHRONOUS type which starts the simulation immediately in the background and then reports it as RUNNING until it is completed. Once completed or if results is called, the state will be SUCCESS.
dstrain115
requested review from
cduck,
vtomole,
wcourtney and
a team
as code owners
| @@ -55,6 +57,10 @@ def __init__( | |||
| self._type = simulation_type | |||
| self._failure_code = '' | |||
| self._failure_message = '' | |||
| if self._type == LocalSimulationType.ASYNCHRONOUS: | |||
| # If asynchronous mode, just kick off a new task and move on. | |||
| self._thread = concurrent.futures.ThreadPoolExecutor(max_workers=1) | |||
There was a problem hiding this comment.
should we be concerned about closing this threadpool? I usually see it used as a context manager which means there should be some sort of try..finally
There was a problem hiding this comment.
Added a call to shutdown immediately, since this will clean up the resources once the future is complete.
| @@ -98,22 +104,31 @@ def batched_results(self) -> List[List[cirq.Result]]: | |||
| raise e | |||
| raise ValueError('Unsupported simulation type {self._type}') | |||
|
|
|||
| def spawn_results(self): | |||
There was a problem hiding this comment.
docstring? why is this called spawn?
There was a problem hiding this comment.
Added docstring and changed this to _execute_results since it should be private.
|
Any further comments on this? |
| @@ -98,22 +110,40 @@ def batched_results(self) -> Sequence[Sequence[cirq.Result]]: | |||
| raise e | |||
| raise ValueError('Unsupported simulation type {self._type}') | |||
|
|
|||
| def _execute_results(self) -> Sequence[cirq.Result]: | |||
There was a problem hiding this comment.
This method may be called by a separate thread if running asynchronously, which introduces potential thread safety issues for other threads that may be polling the state. Is this a concern?
There was a problem hiding this comment.
What thread safety issues are you worried about? Do you think State should be protected by a lock of some sort?Assignment here should be atomic.
* Add asynchronous ability to simulated engine. Adds ASYNCHRONOUS type which starts the simulation immediately in the background and then reports it as RUNNING until it is completed. Once completed or if results is called, the state will be SUCCESS.
Adds ASYNCHRONOUS type which starts the simulation
immediately in the background and then reports it
as RUNNING until it is completed. Once completed
or if results is called, the state will be SUCCESS.