Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support CircuitOperation(use_repetition_ids=False) #533

Open
95-martin-orion opened this issue Jun 6, 2022 · 4 comments
Open

Support CircuitOperation(use_repetition_ids=False) #533

95-martin-orion opened this issue Jun 6, 2022 · 4 comments

Comments

@95-martin-orion
Copy link
Collaborator

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

@maffoo
Copy link
Contributor

maffoo commented Jun 6, 2022

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]: {}

@95-martin-orion
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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.

@maffoo
Copy link
Contributor

maffoo commented Jun 6, 2022

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants