Add support for sampling from Quokka devices #208
Conversation
dstrain115
commented
Sep 3, 2024
dstrain115
commented
- This sampler converts circuits to QASM and then sends them to a quokka endpoint for simulation.
- Any parameterized circuits are resolved and sent point by point to the device.
- This sampler converts circuits to QASM and then sends them to a quokka endpoint for simulation. - Any parameterized circuits are resolved and sent point by point to the device.
unitary/alpha/quokka_sampler.py
Outdated
| "Please install requests library to use Quokka" | ||
| "(e.g. pip install requests)" | ||
| ) | ||
| raise e |
There was a problem hiding this comment.
It might be nicer to augment the exception with a message:
raise ImportError("Please install requests library to use Quokka (e.g. pip install requests)") from e
unitary/alpha/quokka_sampler.py
Outdated
|
|
||
| def run_sweep( | ||
| self, | ||
| program: "cirq.AbstractCircuit", |
There was a problem hiding this comment.
cirq is already imported, are quotes needed for the three cirq types here?
unitary/alpha/quokka_sampler.py
Outdated
| print( | ||
| "Warning! Keys can only be measured once in Quokka simulator" | ||
| ) | ||
| print("Key {key} will only contain the last measured value") |
There was a problem hiding this comment.
add file=sys.stderr to prints if these are warnings?
There was a problem hiding this comment.
Changed to warnings.warn.
unitary/alpha/quokka_sampler.py
Outdated
| if self.endpoint is None: | ||
| self.endpoint = _REQUEST_ENDPOINT.format(self.quokka_name) | ||
| if self.post_function is None: | ||
| self.post_function = self._post |
There was a problem hiding this comment.
this, combined with the _post method below, is a bit weird. A cleaner implementation would be to extract _post method into the module level function, and then pass it as a default value of post_function argument.
Maybe something like that:
class Quokka(object):
def __init__(self, name=_DEFAULT_QUOKKA_NAME):
self._end_point = _REQUEST_ENDPOINT.format(name)
def __call__(self, json_request: JSON_TYPE) -> JSON_TYPE:
result = requests.post(self._endpoint, json=json_request)
return json.loads(result.content)and then
class QuokkaSampler(cirq.Sampler):
def __init__(self, post: Optional[Callable[[JSON_TYPE], JSON_TYPE]] = None):
self._post = post or Quokka()Now there's a clean way to instantiate sampler with different post methods:
s = QuokkaSampler() # production instance
s = QuokkaSampler(post=_mock_post) # unittest post function
s = QuokkaSampler(post=Quokka("staging")) # staging quokka
s = QuokkaSampler(post=Quokka("foo")) # foo quokkain fact, you're already doing something very similar in test, so this refactoring makes even more sense :)
There was a problem hiding this comment.
Done. Kept name as a parameter though since QuokkaSample("foo") seemed cleaner.
unitary/alpha/quokka_sampler.py
Outdated
| for key in measure_keys: | ||
| register_name = register_names[key] | ||
| if register_name not in json_results[_RESULT_KEY]: | ||
| raise RuntimeError( |
unitary/alpha/quokka_sampler_test.py
Outdated
|
|
||
|
|
||
| class FakeQuokkaEndpoint: | ||
| def __init__(self, responses: Iterable[quokka_sampler.JSON_TYPE]): |
There was a problem hiding this comment.
for slightly nicer interface you can do *responses: quokka_sampler.JSON_TYPE and dispense with some square brackets below when passing expected responses. This would also remove the need for list conversion below
There was a problem hiding this comment.
Done. Kept the list conversion so that I don't need to keep track of a counter or iterator.
unitary/alpha/quokka_sampler_test.py
Outdated
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| from typing import Iterable |
There was a problem hiding this comment.
this is not needed anymore
