potential circuit insert issue

[alexandrupaler]

When inserting gates with Circuit.insert(), the gates are (often) added to the previous moment. I assume the fault is in _pick_or_create_inserted_op_moment_index

To replicate the error, the following code shows the problem. I start with a circuit of three CNOTs (one per moment), remove one of the CNOTS (expect three moments with one being empty), insert the same gate back to the Moment index_to_clean, but the gate is added to the previous Moment.

For the following code I get a strange result

import cirq

q = [cirq.NamedQubit("q" + str(i)) for i in range(4)]
circ = cirq.Circuit()
index_to_clean = 1

cnots = [
    cirq.ops.CNOT.on(q[0], q[3]),
    cirq.ops.CNOT.on(q[1], q[2]),
    cirq.ops.CNOT.on(q[1], q[2])
]
circ.append(cnots, strategy=cirq.InsertStrategy.NEW)

print("Fresh circuit", repr(circ))

circ.clear_operations_touching(cnots[index_to_clean].qubits, [index_to_clean])
print("Removed CNOT at {}".format(index_to_clean), repr(circ))

circ.insert(index_to_clean, cnots[index_to_clean], strategy=cirq.InsertStrategy.INLINE)
print("Inserted CNOT at {}".format(index_to_clean), repr(circ))

The output is

Fresh circuit cirq.Circuit([
    cirq.Moment(operations=[
        cirq.CNOT.on(cirq.NamedQubit('q0'), cirq.NamedQubit('q3')),
    ]),
    cirq.Moment(operations=[
        cirq.CNOT.on(cirq.NamedQubit('q1'), cirq.NamedQubit('q2')),
    ]),
    cirq.Moment(operations=[
        cirq.CNOT.on(cirq.NamedQubit('q1'), cirq.NamedQubit('q2')),
    ]),
])
Removed CNOT at 1 cirq.Circuit([
    cirq.Moment(operations=[
        cirq.CNOT.on(cirq.NamedQubit('q0'), cirq.NamedQubit('q3')),
    ]),
    cirq.Moment(),
    cirq.Moment(operations=[
        cirq.CNOT.on(cirq.NamedQubit('q1'), cirq.NamedQubit('q2')),
    ]),
])
Inserted CNOT at 1 cirq.Circuit([
    cirq.Moment(operations=[
        cirq.CNOT.on(cirq.NamedQubit('q0'), cirq.NamedQubit('q3')),
        cirq.CNOT.on(cirq.NamedQubit('q1'), cirq.NamedQubit('q2')),
    ]),
    cirq.Moment(),
    cirq.Moment(operations=[
        cirq.CNOT.on(cirq.NamedQubit('q1'), cirq.NamedQubit('q2')),
    ]),
])

[bryano]

This is the correct, documented behavior.

[mpharrigan]

I was also very surprised when I discovered this behavior #2486

[alexandrupaler]

@bryano The behaviour is documented for Inline, but earliest starts also at index-1? It is also more than confusing.

[bryano]

@alexandrupaler I'm not sure what you mean by "index-1". Could you give an example?

[alexandrupaler]

@bryano: if I call insert(index, strategy=Earliest) the insertion is also performed starting with index-1. For example, if I specify index=4, then insertion starts from index 3 and not 4.

[bryano]

That shouldn't be the general behavior for EARLIEST. For example, if
circuit.insert(index, op, strategy=InsertStrategy.EARLIEST)
inserts op at index inserted_index and there are no operations touching op.qubits in later moments (>= index), then
circuit.insert(index + k, op, strategy=InsertStrategy.EARLIEST)
should insert op at the same inserted_index for any k >= 0.

[dabacon]

I think this is working as intended with the confusing (but python inspired) insert at index being interpreted as the boundary a la the discussion #2486 If I'm wrong, please reopen.

This does point out as described by #2775 how confusing it is to have insertion strategies in our insertion methods.