Refactor [NoiseModelFrom]NoiseProperties #4866
Conversation
95-martin-orion
requested review from
cduck,
vtomole,
wcourtney and
a team
as code owners
There was a problem hiding this comment.
Looks good with some small nits. Also wondering why exactly does the noisy_moments function need this measurement splitting + noise add + measurement recombine stuff ? Why can't it look more like:
noisy_circuit = split_measure_circuit.copy()
for model in self.noise_models:
noisy_circuit = noisy_circuit.with_noise(model)and leave the NoiseModel to look after what's physical and what's virtual etc.
| Device-specific subclasses should implement this method to mark any | ||
| operations which their device handles outside the quantum hardware. | ||
| """ | ||
| return False |
There was a problem hiding this comment.
Should this be False always ?
There was a problem hiding this comment.
In the general case, yes - outside of the Google device (which gets its own implementation of this), what gates are "virtual" varies between devices. I suppose we could check for VirtualTag here, but note that we don't use VirtualTag in the Google version (see google_noise_properties in #4733).
There was a problem hiding this comment.
Discussed offline: keep the current behavior, but document it thoroughly (especially the virtual/non-virtual distinction).
| # Only subclasses will trigger this case. | ||
| if virtual_ops: | ||
| new_moments.append(ops.Moment(virtual_ops)) # coverage: ignore |
There was a problem hiding this comment.
Is this because we expect subclasses to override what's virtual and what is not ?
There was a problem hiding this comment.
Yup, both on the comment and the coverage: ignore.
There was a problem hiding this comment.
@MichaelBroughton I'm not entirely clear on what you're suggesting for measurement splitting, but it exists here for a couple of reasons:
- Populating an
InsertionNoiseModelfor all possible multi-qubit measurement gates is infeasible, since it uses a map of{base_gate: noise}instead of a function. - Handling "what is virtual" here prevents code duplication across noise models.
MichaelBroughton
added
automerge
95-martin-orion
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
|
Automerge cancelled: A required status check is not present. Missing statuses: ['Pytest Ubuntu (3.8)', 'Pytest Ubuntu (3.9)', 'Pytest Windows (3.7)', 'Pytest Windows (3.8)', 'Pytest Windows (3.9)', 'Typescript lint check', 'Typescript tests', 'Typescript tests coverage'] |
CirqBot
removed
automerge
* Update NoiseProperties class * Update calibration-to-noise experiment * Align with naming conventions * Split off SuperconductingQubitNoiseProperties * Two-qubit pauli err from calibrations * Format fix * Resolve trailing TODOs * Reduce to noise_properties. * Review comments * Document (non)virtual behaviors. * Align with moment move Co-authored-by: Cirq Bot <[email protected]>
* Update NoiseProperties class * Update calibration-to-noise experiment * Align with naming conventions * Split off SuperconductingQubitNoiseProperties * Two-qubit pauli err from calibrations * Format fix * Resolve trailing TODOs * Reduce to noise_properties. * Review comments * Document (non)virtual behaviors. * Align with moment move Co-authored-by: Cirq Bot <[email protected]>
This PR converts
NoisePropertiesto an interface and modifiesNoiseModelFromNoisePropertiesto match. It also temporarily removes thecalibration_to_noise_propertiesandcompare_generated_noise_to_metricsfiles which depended on the old versions of this code; these will be reinstated in a later PR.This is part 1 of #4666. Remaining steps include:
SuperconductingQubitsNoisePropertiescalibration_to_noise_propertiesusing new typescompare_generated_noise_to_metricsusing new types