Insertion noise model #4672
Insertion noise model #4672
Conversation
95-martin-orion
requested review from
cduck,
vtomole and
a team
as code owners
| noise_ops: List['cirq.Operation'] = [] | ||
| for op in moment: | ||
| if self.require_physical_tag and PHYSICAL_GATE_TAG not in op.tags: | ||
| # Only non-virtual gates get noise applied. | ||
| continue | ||
| op_id = OpIdentifier(type(op.gate), *op.qubits) | ||
| if op_id in self.ops_added: | ||
| noise_ops.append(self.ops_added[op_id]) | ||
| continue | ||
| # Find the closest match, if one exists. | ||
| parent_id = OpIdentifier(object, *op.qubits) | ||
| for added_id in self.ops_added: | ||
| if added_id.qubits != parent_id.qubits: | ||
| continue | ||
| if not issubclass(op_id.gate_type, added_id.gate_type): | ||
| continue | ||
| if issubclass(added_id.gate_type, parent_id.gate_type): | ||
| parent_id = added_id | ||
| if parent_id.gate_type != object: | ||
| noise_ops.append(self.ops_added[parent_id]) | ||
| if not noise_ops: | ||
| return [moment] | ||
| if self.prepend: | ||
| return [ops.Moment(noise_ops), moment] | ||
| return [moment, ops.Moment(noise_ops)] |
There was a problem hiding this comment.
This is pretty hard to read. Also, the object-like containment check that falls back to type-like containment check is something gatesets can do for you. Also if there are gates or operations in self.ops_added with overlapping parent types this might have uh-intuitive behavior for the user. Could we tighten this up a bit ?
Maybe something along the lines of:
non_virtual_ops = [op if self.require_physical_tag and PHYSICAL_GATE_TAG not in op.tags for op in moment]
mappings = self.ops_added.items()
converted_ops = [v(*op.qubits) for k,v in mappings for op in non_virtual_ops if op in k]
return converted_ops + momentwhere self.ops_added maps from gatesets to operations ?
There was a problem hiding this comment.
cirq.Gateset is not serializable, which causes complications when interacting with NoiseProperties. It's likely possible to get this working, but I expect it to be more complicated than the current behavior.
RE overlapping parent types: this is addressed here and covered in tests. If you have something like:
class A: {...}
class B(A): {...}
class C(B): {...}
ops_added = {
OpIdentifier(A, q0): cirq.X(q0),
OpIdentifier(B, q0): cirq.Z(q0),
}
then objects of type B or C will generate cirq.Z(q0) as noise, and objects of type A will produce cirq.X(q0) as noise.
There was a problem hiding this comment.
cirq.Gatesetis not serializable, which causes complications when interacting withNoiseProperties
Can this be fixed easily @tanujkhattar ?
There was a problem hiding this comment.
Discussed offline. @95-martin-orion will try to replace the existing logic with gate families and gatesets. I will add 2 new features to gatesets
- A method that can return the matched gate family instead of just returning true/false while checking for containment
- If all gate families of a Gateset are serializable, then the gateset should be serializable.
There was a problem hiding this comment.
Discussed out-of-band with @tanujkhattar - I can reconstruct this to use Gatesets and GateFamilys with a fairly low attached cost. I'll probably need to rehash these PRs again to support that, but until then I'm happy to take any other comments you have, since the remaining structure of this change should remain similar.
There was a problem hiding this comment.
Changes have been applied to #4671, and will be added here soon. PTAL
95-martin-orion
force-pushed
the
cirq-noisegen-4b
branch
from
a24de0b to
224ee36
Compare
|
Updated with the new OpIdentifier match tools. Pinging @MichaelBroughton for review. |
|
|
||
| def noisy_moment( | ||
| self, moment: 'cirq.Moment', system_qubits: Sequence['cirq.Qid'] | ||
| ) -> 'cirq.OP_TREE': |
There was a problem hiding this comment.
This defines the noisy_moment method of the parent class and inherits its docstring. Compare with noisy_moments in ConstantQubitNoiseModel.
95-martin-orion
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
This PR is part of quantumlib#4640. It adds the `InsertionNoiseModel`, which injects noise based on a user-defined map. quantumlib#4671 is a prerequisite for this PR. The only files that need to be reviewed in this PR are: - `cirq-core/cirq/devices/...` - `__init__.py` - `insertion_noise_model[_test].py`
This PR is part of quantumlib#4640. It adds the `InsertionNoiseModel`, which injects noise based on a user-defined map. quantumlib#4671 is a prerequisite for this PR. The only files that need to be reviewed in this PR are: - `cirq-core/cirq/devices/...` - `__init__.py` - `insertion_noise_model[_test].py`
This PR is part of quantumlib#4640. It adds the `InsertionNoiseModel`, which injects noise based on a user-defined map. quantumlib#4671 is a prerequisite for this PR. The only files that need to be reviewed in this PR are: - `cirq-core/cirq/devices/...` - `__init__.py` - `insertion_noise_model[_test].py`
This PR is part of #4640. It adds the
InsertionNoiseModel, which injects noise based on a user-defined map.#4671 is a prerequisite for this PR. The only files that need to be reviewed in this PR are:
cirq-core/cirq/devices/...__init__.pyinsertion_noise_model[_test].py