Giving inconsistent measurement key qubits gives unequal repr

[Strilanc]

Ran into this:

import cirq
op = cirq.GateOperation(
    gate=cirq.MeasurementGate(
        1, cirq.MeasurementKey(path=(), name='q0_1_0', qubits=()), ()
    ),
    qubits=[cirq.GridQubit(0, 1)]
)
assert op == eval(repr(op))  # fails

It took me an hour to debug that it was the repr lying to me. Very frustrating. Either the repr needs to correctly represent the mkey qubits differing from the operation qubits, or the creation of the object needs to fail.

I'm also worried about the fact that we apparently introduced a whole new way for things to be inconsistent, by allowing the measurement key to mention qubits.

It looks like the issue is that MeasurementGate.on rekeys the measurement key to target certain qubits, but that's not the only way to apply a measurement gate to qubits, so the rekeying is inconsistent between various code paths.

[95-martin-orion]

@smitsanghavi can you take a look at this? Related: #4040

[Strilanc]

I'll also note that forcing the key to change as part of putting it into a measurement is not ideal because it means certain circuit rewrites become impossible. What if I want to rewrite the circuit in a way that moves the measurement to other qubits, but I want to preserve the key for compatibility with the original circuit?

[95-martin-orion]

Given that @smitsanghavi is currently out of office and it may take some time to come to a good solution on this, I recommend we revert the latest PR in the MeasurementKey sequence and work forwards from there.

[smitsanghavi]

I agree this is an issue. I think it stems from the fact that in direct construction of GateOperation we do not call on on the gate (unlike in cirq.measure) and so the qubits are not passed to the MeasurementKey but op_repr assumes that qubits have been passed.

I'll take a closer look at this (and possibly reevaluate the requirements) when I am back but reverting sounds perfect meanwhile.

[95-martin-orion]

#4277 fixes this by reverting the addition of the qubit field to MeasurementKey. Prior to that change, MeasurementKey was a simple wrapper around string keys which did not experience this issue.