Ensure simulator runs circuit compatible with a provided noise model's properties - #8139
Acciaccatura wants to merge 12 commits into
Conversation
|
question - this change seems to be blocked on a Docstring test failure, specifically this one: however when checking the expected gateset of the noise properties from this processor myself, it seems its failure is working as intended according to my implementation: >>> import cirq_google
>>> cal = cirq_google.engine.load_median_device_calibration("rainbow")
>>> noise_props = cirq_google.engine.noise_properties_from_calibration(cal, gate_times_ns="legacy\
")
>>> print(noise_props.expected_gates())
{<class 'cirq.ops.measurement_gate.MeasurementGate'>, <class 'cirq.ops.swap_gates.ISwapPowGate'>, <class 'cirq.ops.fsim_gate.FSimGate'>, <class 'cirq.ops.common_gates.ZPowGate'>, <class 'cirq.ops.fsim_gate.PhasedFSimGate'>, <class 'cirq.ops.common_gates.CZPowGate'>, <class 'cirq.ops.phased_x_z_gate.PhasedXZGate'>, <class 'cirq_google.ops.sycamore_gate.SycamoreGate'>, <class 'cirq.ops.common_channels.ResetChannel'>}(i.e. my implementation assumes |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8139 +/- ##
=======================================
Coverage 99.59% 99.59%
=======================================
Files 1131 1131
Lines 103601 103642 +41
=======================================
+ Hits 103180 103221 +41
Misses 421 421 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| unexpected_qubits = [cirq.GridQubit(2, 2)] | ||
|
|
||
| class TestNoiseProperties(devices.SuperconductingQubitsNoiseProperties): |
There was a problem hiding this comment.
the TestNoiseProperties created for this test might not make sense from a QC point of view just because of my limited domain knowledge - feel free to re-implement to something more sensible if this is a concern!
|
@Acciaccatura Thank you for this effort. If you are still working on this, would you mind changing it to a draft PR? |
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
450e75f to
7e8f89d
Compare
|
hey @mhucka ! sorry I missed your message. this PR is ready for review - I just rebased :). i've left some comments on some areas I think might require some clarification. |
| circuit_gates = {op.gate for op in circuit.all_operations()} | ||
| if not all( | ||
| any( | ||
| isinstance(gate, expected_gate) |
There was a problem hiding this comment.
noise models define their expected gateset explicitly - for example:
Cirq/cirq-google/cirq_google/devices/google_noise_properties.py
Lines 189 to 191 in 4adcfb9
there are some gates that are mathematically equivalent to other gates (e.g. I think PhasedXZGate(axis_phase_exponent=0, x_exponent=1, z_exponent=0) and X), but I believe this check will not allow X. is this ok, and if not is there a better alternative for testing gate equality?
When provided a
NoiseModelFromNoisePropertiesnoise model to simulate, ensure the circuit uses qubits and gates that are compatible with the noise properties. At a slightly lower level:NoiseModelFromNoisePropertiesexposes its underlyingNoisePropertiesas a public property.SimulatorBasewill check to make sure all the gates and qubits used by the circuit are permitted under anyNoisePropertiesfrom the associatedNoiseModelFromNoiseProperties.SuperconductingQubitsNoisePropertiescontainsexpected_qubitsorexpected_gates, so the code is currently hardcoded to this class. I'm uncertain if Cirq has any plans on genericizing this behavior toNoiseProperties.fixes #6608