Cirq icon indicating copy to clipboard operation
Cirq copied to clipboard

`drop_terminal_measurements` fails for measurements on dimension>2 Qids

Open richrines1 opened this issue 2 years ago • 8 comments

Description of the issue

the cirq.drop_terminal_measurements transformer fails if any measurements are on Qids with dimension greater than 2

How to reproduce the issue

_ = cirq.drop_terminal_measurements(cirq.Circuit(cirq.measure(cirq.LineQid(0, dimension=2))))  # ok
_ = cirq.drop_terminal_measurements(cirq.Circuit(cirq.measure(cirq.LineQid(0, dimension=3))))  # ValueError: Wrong shape of qids for <cirq.I>. Expected (2,) but got (3,) <[cirq.LineQid(0, dimension=3)]>.

Cirq version

1.2.0.dev20230105212249

richrines1 avatar Feb 02 '23 18:02 richrines1

it looks like this arises when inserting X/I operations according to invert_mask - out of curiosity how should invert_mask be interpreted for qudit measurements?

richrines1 avatar Feb 08 '23 17:02 richrines1

Adding to triage discuss to answer above question. How should we handle invert_mask when using qudits?

dstrain115 avatar Feb 15 '23 17:02 dstrain115

To be compatible with deferred_measurement_transformer it has to be equivalent to applying an X on that qudit, e.g. a mod_add(1). (Or if something else is decided, then the deferred_measurement_transformer needs to be updated to match). https://github.com/quantumlib/Cirq/blob/27dd6076f8fb9631c20d6234134edd76c9cdd681/cirq-core/cirq/transformers/measurement_transformers.py#L116

daxfohl avatar Feb 17 '23 05:02 daxfohl

I take that back, the deferred_measurement_transformer code doesn't handle it at all. It'd just require changing X to XPowGate and explicitly passing in the dimension though. I think this is the best option nonetheless. If there's a desire to allow int in the invert_mask, for completions sake, that would translate to an exponent in deferred_measurement_transformer's XPowGate.

daxfohl avatar Feb 17 '23 05:02 daxfohl

i think that's probably what i'd expect to happen, though also noticing that the simulator currently does something different (applying ^= 1 to the result iff the measured value is either 0 or 1)

also fwiw it looks like the same pattern comes up again in cirq.circuits.circuit._decompose_measurement_inversions, leading to e.g.,

measurement = cirq.measure(cirq.LineQubit(0, dimension=3), invert_mask=(True,))
cirq.has_unitary(cirq.Circuit(measurement))  # ValueError: Wrong shape of qids for <cirq.X>. Expected (2,) but got (3,) <[cirq.LineQid(0, dimension=3)]>.

richrines1 avatar Feb 17 '23 07:02 richrines1

invert_mask is supposed to be a classical post processing on the measurement outcomes of a qubit which conveys that we should "flip" the measurement results. As discussed above, extending this to general qids would require us start supporting add_mod(k), so for qutrits one could either add 1 or add 2 (both mod 3) to the measurement results.

However, note that we already support confusion_map parameter in MeasurementGate, which provides a more general way to specify what classical post processing should be applied to the measurement results. Note that confuson_map was introduced after invert_mask and invert_mask has been kept around for legacy and backwards compatibility.

Going forward, I recommend we do the following:

  1. Raise an error for general qudits if an invert_mask is specified.
  2. Use confusion_map to specify the classical post processing that should be applied to general qudits.
  3. Potentially deprecate invert_mask for next major release (cirq 2.0).

@daxfohl @richrines1 Let me know what you think.

tanujkhattar avatar Mar 01 '23 22:03 tanujkhattar

Makes sense. invert_mask would be a horrible name for an inc_mod function, even though it's the only reasonable qudit behavior. More likely to confuse people than anything.

I think no need to deprecate it though, it's still a useful and meaningful shortcut.

daxfohl avatar Mar 01 '23 22:03 daxfohl

Hi, is anyone working on this issue, if not I will pick this

madhurimaparuchuri avatar May 02 '23 04:05 madhurimaparuchuri