qsim icon indicating copy to clipboard operation
qsim copied to clipboard

Support CircuitOperation(use_repetition_ids=False)

Open 95-martin-orion opened this issue 2 years ago • 4 comments

Highlighted in #531 - apparently CircuitOperation(use_repetition_ids=False) does not convert properly to qsim due to not finding the associated measurements.

95-martin-orion avatar Jun 06 '22 15:06 95-martin-orion

I think the problem occurs also with use_repetition_ids=True and whether or not the circuit operation has repetitions. Here's a minimal example:

In [1]: q = cirq.q(0)

In [2]: circuit = cirq.Circuit(cirq.measure(q, key='m')); circuit
Out[2]: 0: ───M('m')───

In [3]: circuit2 = cirq.Circuit(circuit.freeze().to_op()); circuit2
Out[3]: 0: ───[ 0: ───M('m')─── ]───

In [4]: import qsimcirq; qsimcirq.__version__
Out[4]: '0.12.2.dev20220422'

In [5]: result = qsimcirq.QSimSimulator().run(circuit, repetitions=10); result
Out[5]: m=0000000000

In [6]: result2 = qsimcirq.QSimSimulator().run(circuit2, repetitions=10); result2
Out[6]: 

In [7]: result2.records
Out[7]: {}

maffoo avatar Jun 06 '22 16:06 maffoo

Yup, looks like we've been relying on findall_operations_with_gate_type, which doesn't bother searching inside of CircuitOperations. Modifying that method is probably the right way to resolve this, although because of how qsimcirq handles the output we won't be able to support repeat-until without additional changes.

95-martin-orion avatar Jun 06 '22 16:06 95-martin-orion

One additional issue: findall_operations_with_gate_type returns a (moment index, op, gate) tuple. For a sub-operation of a CircuitOperation, moment index doesn't precisely locate the operation.

95-martin-orion avatar Jun 06 '22 16:06 95-martin-orion

I think some specific logic for finding measurement operations would be generically useful. For example, in cirq-google we have a find_measurements function to find measurements in a circuit which does something similar to what we need here (that function also needs to be updated to handle CircuitOperation).

maffoo avatar Jun 06 '22 17:06 maffoo