Allow creating new gatesets with added gates #2458
Conversation
| self.deserializers = {d.serialized_gate_id: d for d in deserializers} | ||
|
|
||
| def with_gates( | ||
| self, | ||
| *, |
There was a problem hiding this comment.
Why do we need this asterisk? Is there a reason to have an empty args?
There was a problem hiding this comment.
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?
| return SerializableGateSet( | ||
| name or self.gate_set_name, | ||
| serializers=[*self._all_serializers(), *serializers], | ||
| deserializers=[*self.deserializers.values(), *deserializers]) |
There was a problem hiding this comment.
Why do we need _all_serializers() but not deserializers?
There was a problem hiding this comment.
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 .values(). I will refactor to just inline the expression, since it's probably not worth having a whole method for the serializer flattening.
cirq/google/serializable_gate_set.py
Outdated
| self.deserializers = {d.serialized_gate_id: d for d in deserializers} | ||
|
|
||
| def with_gates( |
There was a problem hiding this comment.
This function needs tests.
maffoo
changed the title
cirq/google/serializable_gate_set.py
Outdated
| self.deserializers = {d.serialized_gate_id: d for d in deserializers} | ||
|
|
||
| def with_gates( |
There was a problem hiding this comment.
Rename this to with_more_gates or something similar, to make it clearer that the original gates are not being discarded.
There was a problem hiding this comment.
Changed to with_added_gates.
maffoo
changed the title
Strilanc
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
the
automerge
CirqBot
removed
the
front_of_queue_automerge
This will let us create "pre-release" gatesets to deploy server-side support for new gates that are not yet supported by the API.