Remove use of .device in cirq-google.
#4820
Conversation
MichaelBroughton
requested review from
cduck,
vtomole,
wcourtney and
a team
as code owners
| @@ -634,7 +634,6 @@ def _serialize_program( | |||
| ) -> any_pb2.Any: | |||
| if not isinstance(program, cirq.AbstractCircuit): | |||
| raise TypeError(f'Unrecognized program type: {type(program)}') | |||
| program.device.validate_circuit(program) | |||
There was a problem hiding this comment.
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.
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.
Added to the list thx.
| 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.
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.
| if device is None: | ||
| raise ValueError('Deserializing schedule requires a device but None was given.') |
There was a problem hiding this comment.
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.
Schedule has been deprecated for over 2.5 years, so we don't need to worry about breakage here.
MichaelBroughton
added
size: M
There was a problem hiding this comment.
I think this is fine. Most of the changes are in deprecated v1/schedule code. so that's no big deal (we should consider deleting some/all of it actually).
Not validating in the engine code is the only concern, but that seems like an unusual operation to do anyway.
| @@ -634,7 +634,6 @@ def _serialize_program( | |||
| ) -> any_pb2.Any: | |||
| if not isinstance(program, cirq.AbstractCircuit): | |||
| raise TypeError(f'Unrecognized program type: {type(program)}') | |||
| program.device.validate_circuit(program) | |||
There was a problem hiding this comment.
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?
| if device is None: | ||
| raise ValueError('Deserializing schedule requires a device but None was given.') |
There was a problem hiding this comment.
Schedule has been deprecated for over 2.5 years, so we don't need to worry about breakage here.
MichaelBroughton
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
Next step in quantumlib#4744 . Removes use of device attachment ability in cirq-google. This is a breaking change in the sense that some methods become "more forgiving" which I've highlighted below. Happy to add the tag if people think it's appropriate to label it.
Next step in quantumlib#4744 . Removes use of device attachment ability in cirq-google. This is a breaking change in the sense that some methods become "more forgiving" which I've highlighted below. Happy to add the tag if people think it's appropriate to label it.
Next step in quantumlib#4744 . Removes use of device attachment ability in cirq-google. This is a breaking change in the sense that some methods become "more forgiving" which I've highlighted below. Happy to add the tag if people think it's appropriate to label it.
Next step in #4744 . Removes use of device attachment ability in cirq-google. This is a breaking change in the sense that some methods become "more forgiving" which I've highlighted below. Happy to add the tag if people think it's appropriate to label it.