-
Notifications
You must be signed in to change notification settings - Fork 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
Allow creating new gatesets with added gates #2458
Changes from all commits
8e14e36
114188e
2aab965
7fa3230
6849469
02c6268
56b914f
c4c8c58
6bf541f
00a65fc
0aca2ae
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 |
---|---|---|
|
@@ -13,10 +13,8 @@ | |
# limitations under the License. | ||
"""Support for serializing and deserializing cirq.api.google.v2 protos.""" | ||
|
||
from collections import defaultdict | ||
|
||
from typing import cast, Dict, Iterable, List, Optional, Tuple, Type, Union, \ | ||
TYPE_CHECKING | ||
from typing import (cast, Dict, Iterable, List, Optional, Tuple, Type, Union, | ||
TYPE_CHECKING) | ||
|
||
from google.protobuf import json_format | ||
|
||
|
@@ -51,12 +49,35 @@ def __init__(self, gate_set_name: str, | |
forms of gates to GateOperations. | ||
""" | ||
self.gate_set_name = gate_set_name | ||
self.serializers = defaultdict( | ||
list) # type: Dict[Type, List[op_serializer.GateOpSerializer]] | ||
self.serializers: Dict[Type, List[op_serializer.GateOpSerializer]] = {} | ||
for s in serializers: | ||
self.serializers[s.gate_type].append(s) | ||
self.serializers.setdefault(s.gate_type, []).append(s) | ||
self.deserializers = {d.serialized_gate_id: d for d in deserializers} | ||
|
||
def with_added_gates( | ||
self, | ||
*, | ||
gate_set_name: Optional[str] = None, | ||
serializers: Iterable[op_serializer.GateOpSerializer] = (), | ||
deserializers: Iterable[op_deserializer.GateOpDeserializer] = (), | ||
) -> 'SerializableGateSet': | ||
"""Creates a new gateset with additional (de)serializers. | ||
|
||
Args: | ||
gate_set_name: Optional new name of the gateset. If not given, use | ||
the same name as this gateset. | ||
serializers: Serializers to add to those in this gateset. | ||
deserializers: Deserializers to add to those in this gateset. | ||
""" | ||
# Iterate over all serializers in this gateset. | ||
curr_serializers = (serializer | ||
for serializers in self.serializers.values() | ||
for serializer in serializers) | ||
return SerializableGateSet( | ||
gate_set_name or self.gate_set_name, | ||
serializers=[*curr_serializers, *serializers], | ||
deserializers=[*self.deserializers.values(), *deserializers]) | ||
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. Why do we need _all_serializers() but not deserializers? 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. Serializers are stored as a map from gate_id to list of serializers, so we have to flatten those into one list. Deserializers are stored as a map from gate id to deserializer, so we can just use |
||
|
||
def supported_gate_types(self) -> Tuple: | ||
return tuple(self.serializers.keys()) | ||
|
||
|
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.
Why do we need this asterisk? Is there a reason to have an empty args?
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.
This forces using keyword args, which I think helps clarity. You're correct that caling this with no arguments is not very useful because it just returns an equivalent gateset. I could also make serializers and deserializers be positional args and put the optional name last. Do you have a preference?