-
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
Remove use of .device
in cirq-google.
#4820
Changes from 7 commits
50bebc2
f4d02cd
b0c4235
0b66781
12a1f64
5792e5f
8e4eab1
50cac19
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 |
---|---|---|
|
@@ -21,17 +21,19 @@ | |
import cirq_google | ||
|
||
|
||
@cirq._compat.deprecated_parameter( | ||
deadline='v0.15', | ||
fix=cirq.circuits.circuit._DEVICE_DEP_MESSAGE, | ||
parameter_desc='new_device', | ||
match=lambda args, kwargs: 'new_device' in kwargs, | ||
) | ||
def optimized_for_xmon( | ||
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. Plan to deprecate the device function signature from this optimizer because it is not a device aware optimizer and all the device was used for was attaching it to the returned circuit. |
||
circuit: cirq.Circuit, | ||
new_device: Optional['cirq_google.XmonDevice'] = None, | ||
qubit_map: Callable[[cirq.Qid], cirq.GridQubit] = lambda e: cast(cirq.GridQubit, e), | ||
allow_partial_czs: bool = False, | ||
) -> cirq.Circuit: | ||
if allow_partial_czs: | ||
return optimized_for_sycamore( | ||
circuit, new_device=new_device, qubit_map=qubit_map, optimizer_type='xmon_partial_cz' | ||
) | ||
else: | ||
return optimized_for_sycamore( | ||
circuit, new_device=new_device, qubit_map=qubit_map, optimizer_type='xmon' | ||
) | ||
optimizer_type = 'xmon_partial_cz' if allow_partial_czs else 'xmon' | ||
ret = optimized_for_sycamore(circuit, qubit_map=qubit_map, optimizer_type=optimizer_type) | ||
ret._device = new_device or circuit._device | ||
return ret |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -273,6 +273,12 @@ def serialize_circuit_op( | |
return proto_msg | ||
raise ValueError(f'Cannot serialize CircuitOperation {op!r}') | ||
|
||
@cirq._compat.deprecated_parameter( | ||
deadline='v0.15', | ||
fix=cirq.circuits.circuit._DEVICE_DEP_MESSAGE, | ||
parameter_desc='device', | ||
match=lambda args, kwargs: 'device' in kwargs or len(args) > 2, | ||
) | ||
def deserialize( | ||
self, proto: v2.program_pb2.Program, device: Optional[cirq.Device] = None | ||
) -> cirq.Circuit: | ||
|
@@ -322,10 +328,10 @@ def deserialize( | |
constants=proto.constants, | ||
deserialized_constants=deserialized_constants, | ||
) | ||
return circuit if device is None else circuit.with_device(device) | ||
if device is not None: | ||
circuit._device = device # coverage: ignore | ||
return circuit | ||
if which == 'schedule': | ||
if device is None: | ||
raise ValueError('Deserializing schedule requires a device but None was given.') | ||
Comment on lines
-327
to
-328
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. There is a break here that schedules can now have no device. Since we aren't using these anymore I figured this was an acceptable vendor break. @dstrain115 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. Schedule has been deprecated for over 2.5 years, so we don't need to worry about breakage here. 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. SG |
||
return self._deserialize_schedule( | ||
proto.schedule, device, arg_function_language=proto.language.arg_function_language | ||
) | ||
|
@@ -497,7 +503,7 @@ def _deserialize_circuit( | |
def _deserialize_schedule( | ||
self, | ||
schedule_proto: v2.program_pb2.Schedule, | ||
device: cirq.Device, | ||
device: Optional[cirq.Device], | ||
*, | ||
arg_function_language: str, | ||
) -> cirq.Circuit: | ||
|
@@ -510,4 +516,8 @@ def _deserialize_schedule( | |
scheduled_op_proto.operation, arg_function_language=arg_function_language | ||
) | ||
) | ||
return cirq.Circuit(result, device=device) | ||
ret = cirq.Circuit(result) | ||
if device is None: | ||
device = cirq.UNCONSTRAINED_DEVICE | ||
ret._device = device | ||
return ret |
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.
Engine no longer validates inside of serialization. As we know this already wasn't doing a great job of catching all the breaks that might come over through engine. Would probably make more sense to couple this up with your new work on the engine emulator. What are your thoughts here @dstrain115 ?
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.
I think this is okay, but we should probably modify documentation to make sure that people validate independently of the serialization then. Maybe we could add this to the cirq 1.0 doc list?
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.
Added to the list thx.