-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Support for stimcirq gates and operations in cirq_google protos #7101
Changes from 5 commits
83ef921
7dc35df
f46b6ed
2957141
728057d
edddb0c
b91c07c
96b832c
31e59d2
867816b
36dd7ab
73c16e1
9f76b20
b96e5dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -191,7 +191,6 @@ def _serialize_gate_op( | |
ValueError: If the operation cannot be serialized. | ||
""" | ||
gate = op.gate | ||
|
||
if isinstance(gate, InternalGate): | ||
arg_func_langs.internal_gate_arg_to_proto(gate, out=msg.internalgate) | ||
elif isinstance(gate, cirq.XPowGate): | ||
|
@@ -258,6 +257,23 @@ def _serialize_gate_op( | |
arg_func_langs.float_arg_to_proto( | ||
gate.q1_detune_mhz, out=msg.couplerpulsegate.q1_detune_mhz | ||
) | ||
elif op.__module__.startswith('stimcirq') or ( | ||
gate is not None and gate.__module__.startswith('stimcirq') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can replace this entire line with "getattr(gate, "module", "")" since it handles There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea, done. |
||
): | ||
# Special handling for stimcirq objects, which can be both operations and gates. | ||
stimcirq_obj = op if op.__module__.startswith('stimcirq') else gate | ||
if stimcirq_obj is not None and hasattr(stimcirq_obj, '_json_dict_'): | ||
# All stimcirq gates currently have _json_dict_defined | ||
msg.internalgate.name = type(stimcirq_obj).__name__ | ||
msg.internalgate.module = 'stimcirq' | ||
if isinstance(stimcirq_obj, cirq.Gate): | ||
msg.internalgate.num_qubits = stimcirq_obj.num_qubits() | ||
else: | ||
msg.internalgate.num_qubits = len(stimcirq_obj.qubits) | ||
|
||
# Store json_dict objects in gate_args | ||
for k, v in stimcirq_obj._json_dict_().items(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of relying on the stimcirq object to always have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure I understand what you are proposing. How do we know which attributes to get from the stimcirq object otherwise? Also, how do we know how to instantiate the appropriate object on deserialization. I think saying that a stimcirq gate/operation must have a json_dict to be imported/exported by cirq_google protos is probably a reasonable requirement (plus, we control both repos, so we can add it if it's missing). I did add a value error if this condition is not met though. |
||
arg_func_langs.arg_to_proto(value=v, out=msg.internalgate.gate_args[k]) | ||
else: | ||
raise ValueError(f'Cannot serialize op {op!r} of type {type(gate)}') | ||
|
||
|
@@ -659,7 +675,31 @@ def _deserialize_gate_op( | |
raise ValueError(f"dimensions {dimensions} for ResetChannel must be an integer!") | ||
op = cirq.ResetChannel(dimension=dimensions)(*qubits) | ||
elif which_gate_type == 'internalgate': | ||
op = arg_func_langs.internal_gate_from_proto(operation_proto.internalgate)(*qubits) | ||
parsed_as_stimcirq = False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we move this code to a local There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After factoring out the import, this code block is pretty short. No one gates have a local function. |
||
msg = operation_proto.internalgate | ||
if msg.module == 'stimcirq': | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Can There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
# special handling for stimcirq | ||
try: | ||
import stimcirq | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If stimcirq is not installed this would search sys.path for every deserialized gate. Consider moving this to a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's only for operations with a module name of stimcirq, but, still, this is a good idea, and done. |
||
|
||
# Use JSON resolver to instantiate the object | ||
if msg.name in stimcirq.JSON_RESOLVERS_DICT: | ||
kwargs = {} | ||
for k, v in msg.gate_args.items(): | ||
arg = arg_func_langs.arg_from_proto(v) | ||
if arg is not None: | ||
kwargs[k] = arg | ||
op = stimcirq.JSON_RESOLVERS_DICT[msg.name](**kwargs) | ||
if qubits: | ||
op = op(*qubits) | ||
parsed_as_stimcirq = True | ||
|
||
except ModuleNotFoundError: # pragma: no cover | ||
# fall back to creating internal gates if stimcirq not installed | ||
pass | ||
if not parsed_as_stimcirq: | ||
# all other internal gates | ||
op = arg_func_langs.internal_gate_from_proto(msg)(*qubits) | ||
elif which_gate_type == 'couplerpulsegate': | ||
gate = CouplerPulse( | ||
hold_time=cirq.Duration( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer
getattr(op, "__module__", "")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, done.