MergeSingleQubitGates only merges to the right

[mpharrigan]

I'm trying to merge my single qubit gates without disturbing the overall structure of my circuit.

q0, q1 = cirq.LineQubit.range(2)
circuit = cirq.Circuit(
    cirq.Moment([cirq.X(q0)]),
    cirq.Moment([cirq.Z(q0), cirq.Y(q1)]),
)
circuit

0: ───X───Z───

1: ───────Y───

c2 = circuit.copy()
cirq.merge_single_qubit_gates_into_phased_x_z(c2)
c2

0: ───PhX(0.5)──────────────

1: ──────────────PhX(0.5)───

What I expected: one moment
What I got: two moments

This transformer uses PointOptimizer which only iterates over operations. We should (could?) iterate over circuit coordinates and handle None for the op argument in optimize_at (and in find_until_blocked)

[thanacles]

@mpharrigan Correct me if I'm wrong. I feel like we should be careful what assumptions being made by merge_single_qubit_gates_into_phased_x_z. What should be the outcome of:

0: ───X───────Z───

1: ───────Y───────

Personally, I can see some people wanting it to be flattened to 1 moment, and some people wanting the q1 operation to be in a different moment as the q0 operation. Do you disagree? Also, it looks like there is another optimization that can be used to flatten your use case down to 1 moment namely AlignLeft.

What do you think the result of running merge_single_qubit_gates_into_phased_x_z on the above operation should be?

[mpharrigan]

I don't 100% understand your example. That is not the circuit you get if you run the code you provided.

If you passed in moments to get the diagram you put, then I agree there's some ambiguity. I would say the q1 would be in a different moment.

In my use case, you must imagine that the circuit fragment presented is part of a larger circuit. Doing a blanket AlignLeft could misalign other things where i'm trying to be careful.

[thanacles]

You are right. I (mistakenly) assumed that the operators would get wrapped in moments, I should have made sure before commenting. That said, your assumption of what I meant was correct.

Maybe the ambiguity can be removed by adding an optional boolean parameter specifying to merge right instead of left. What do you think?

Updated my comment above to reflect what I was trying to communicate.

[MichaelBroughton]

@mpharrigan, @tanujkhattar is this still relevant under the new transformers ?

[tanujkhattar]

So, with the new transformer infrastructure, the output of the original circuit would still be the same, i.e.

>>> import cirq
>>> q0, q1 = cirq.LineQubit.range(2)
>>> circuit = cirq.Circuit(
    cirq.Moment([cirq.X(q0)]),
    cirq.Moment([cirq.Z(q0), cirq.Y(q1)]),
)
>>> cirq.merge_single_qubit_gates_to_phased_x_and_z(circuit)
0: ───PhX(0.5)──────────────

1: ──────────────PhX(0.5)───

But, we can now use tags to align specific subset of operations after the merge is done, so a blanket align_left does not destroy the moment structure that we are trying to preserve. For example:

In [1]: q = cirq.LineQubit.range(4)
In [2]: tag = "optimize"
In [3]: circuit = cirq.Circuit(
    ...:     cirq.H(q[3]), cirq.CNOT(q[3], q[2]),
    ...:     cirq.H(q[0]), cirq.CNOT(*q[:2]),
    ...:     cirq.X(q[0]).with_tags(tag),
    ...:     cirq.Moment(cirq.Z(q0).with_tags(tag), cirq.Y(q1).with_tags(tag)),
    ...:     cirq.H(q[3]), cirq.CNOT(q[3], q[2]),
    ...:     cirq.H(q[0]), cirq.CNOT(*q[:2]),
    ...:     strategy=cirq.InsertStrategy.NEW
    ...:     )
In [4]: def optimize_tagged_subset(circuit, tag):
    ...:     circuit = cirq.toggle_tags(circuit, tags=[tag])
    ...:     context = cirq.TransformerContext(tags_to_ignore=[tag])
    ...:     circuit = cirq.merge_single_qubit_gates_to_phased_x_and_z(circuit, context=context)
    ...:     circuit = cirq.align_left(circuit, context=context)
    ...:     circuit = cirq.drop_empty_moments(circuit)
    ...:     return cirq.toggle_tags(circuit, tags = [tag])
In [5]: circuit
Out[5]:
0: ───────────H───@───X['optimize']───Z['optimize']───────────H───@───
                  │                                               │
1: ───────────────X───────────────────Y['optimize']───────────────X───

2: ───────X───────────────────────────────────────────────X───────────
          │                                               │
3: ───H───@───────────────────────────────────────────H───@───────────

In [6]: optimize_tagged_subset(circuit, tag=tag)
Out[6]:
0: ───────────H───@───PhX(0.5)['optimize']───────────H───@───
                  │                                      │
1: ───────────────X───PhX(0.5)['optimize']───────────────X───

2: ───────X──────────────────────────────────────X───────────
          │                                      │
3: ───H───@──────────────────────────────────H───@───────────

I would also argue that changing merge_single_qubit_gates_to_phased_x_and_z to move the merged operation to the left will be pretty non-trivial because we don't always want to move the merged operation to the left (for eg: if the moment on the left is a 2-qubit moment, we would not want to do the merge and move the merged operation to the left).

Given the new features of having more control on optimizing / aligning a subset of gates in the circuit using tags, I'd suggest that we close this issue.

@mpharrigan What do you think?

[dstrain115]

Pinging @mpharrigan again since this is marked as before 1.0. Does the above comment satisfy your concerns?

[tanujkhattar]

@mpharrigan Gentle reminder to take a look at my comment above and close the issue if this meets your requirements.

[mpharrigan]

I mean it seems a little clunky.

In your example you have one "group" of single qubit gates. How would it work if I had e.g. a repeating structure of single- and two-qubit gates (but the single qubit part could be multiple moments as above) and I wanted to merge all the single qubit gates down? Would I need unique tags for each group? Would I need to call align_left for each group?

[tanujkhattar]

How would it work if I had e.g. a repeating structure of single- and two-qubit gates (but the single qubit part could be multiple moments as above) and I wanted to merge all the single qubit gates down?

We have a cirq.merge_single_qubit_moments_to_phxz transformer now; which merges all adjacent moments containing single qubit gates into a single moment containing single qubit gates. This was created to deal with the case where we have alternating layers of single and 2q gates and want to merge all single qubit moments sandwiched between layers of 2q gates.

[mpharrigan]

ok that sounds good

[tanujkhattar]

SG, I'll close the issue then. Thanks.