DensityMatrixSimulator not working with subcircuits

[daxfohl]

Description of the issue

It appears density matrix simulator does not work with subcircuits? After looking through the noise simulation code I was trying to check whether noise propagated through subcircuits but I'm unable to even get a measurement. Entirely possible I'm using this incorrectly, or that this is expected behavior.

Note: if changed to the sparse simulator, the test passes.

How to reproduce the issue

    def test_simulate_noise_with_subcircuits():
        q = cirq.LineQubit(0)
        circuit1 = cirq.Circuit(cirq.measure(q))
        circuit2 = cirq.Circuit(cirq.CircuitOperation(cirq.Circuit(cirq.measure(q)).freeze()))

        simulator = cirq.DensityMatrixSimulator(noise=cirq.X)
        result1 = simulator.run(circuit1, repetitions=10)
        result2 = simulator.run(circuit2, repetitions=10)

>       assert result1 == result2
E       AssertionError: assert cirq.Result(params=cirq.ParamResolver({}), measurements={'0': np.array([[0], [0], [0], [0], [0], [0], [0], [0], [0], [0]], dtype=np.int8)}) == cirq.Result(params=cirq.ParamResolver({}), measurements={})
E         +cirq.Result(params=cirq.ParamResolver({}), measurements={'0': np.array([[0], [0], [0], [0], [0], [0], [0], [0], [0], [0]], dtype=np.int8)})
E         -cirq.Result(params=cirq.ParamResolver({}), measurements={})

Cirq version 0.10.0.dev (latest master)

[smitsanghavi]

I think the root cause is that DensityMatrixSimulator considers only ops.MeasurementGate for sampling. The fix would be to use protocols.is_measurement instead of circuit.findall_operations_with_gate_type(ops.MeasurementGate).

This should apply to (almost?) everything in Cirq using MeasurementGate checks. Related to #3678.

[viathor]

Reproduced.

[daxfohl]

@smitsanghavi thanks, that got past the first error. Now it's failing at the next step, and this one seems thorny. What's happening now is that the subcircuit operation is returning true for protocols.is_measurement. And since the subcircuit is terminal, the iterator just ignores it.

                if protocols.is_measurement(op):
                    measured[op.qubits] = True
                    if all_measurements_are_terminal:
                        continue

Then when it tries to sample all terminal measurement gates

        measurement_ops = [
            op for _, op, _ in circuit.findall_operations(protocols.is_measurement)
        ]
        return step_result.sample_measurement_ops(measurement_ops, repetitions, seed=self._prng)

that subcircuit operation gets included and it crashes.

Seems like we either need a way to sample a subcircuit, or we unroll the circuit before calling _run_sweep_sample, Sampling the subcircuit would probably involve a recursive call to run the simulation on the subcircuit moments.

[daxfohl]

Found a workaround by forcing run_sweep_repeat instead of run_sweep_sample when subcircuits exist.