PauliString class sorts qubits by default, which results in confusing semantics

[tanujkhattar]

Description

PauliString represents a collection of pauli's applied on different qubits. Since pauli operators applied to different qubits commute with each other, the class sorts all the qubits by default. Eg:

In [3]: cirq.X(q[1]) * cirq.Y(q[2]) * cirq.Z(q[0])
Out[3]: Z(0)*X(1)*Y(2) # Instead of X(1) * Y(2) * Z(0)

Sorting qubits is not common in other operations (eg: ControlledOperations, GateOperations etc.), and has some confusing semantics as shown below:

Conversion to/from DensePauliString results in different gates

gate_instance == gate_instance.on().gate() should generally be true for all gates, but this is not the case with DensePauliString. Eg:

In [4]: cirq.DensePauliString('XYZ', coefficient=(1+0j)).on(q[1], q[2], q[0]).gate
Out[4]: cirq.DensePauliString('ZXY', coefficient=(1+0j))

with_qubits returns a different gate applied on different qubits.

with_qubits function is supposed to return the same operations applied to different qubits but the underlying gate also changes with qubit ordering, similar to the same issue as above. Eg:

In [5]: ps = cirq.X(q[0]) * cirq.Y(q[1]) * cirq.Z(q[2])
   ...: ps.gate == ps.with_qubits(q[1], q[2], q[0]).gate
Out[5]: False

Need for a matrix() protocol.

Since changing the ordering of the qubits (which happens implicitly) changes the underlying "gate" represented by the operation, the unitary matrix of the operation is not necessarily the same as what one would expect. To get the expected representation, one needs to use .matrix(), which makes a case for matrix protocol propsoed in #3099. Eg:

In [6]: m_expected = cirq.linalg.kron(*[cirq.unitary(p) for p in [cirq.X, cirq.Y, cirq.Z]])
    ..: m_unitary = cirq.unitary(cirq.X(q[2]) * cirq.Y(q[1]) * cirq.Z(q[0]))
    ..: m_matrix = (cirq.X(q[2]) * cirq.Y(q[1]) * cirq.Z(q[0])).matrix([q[2], q[1], q[0]])
    ..: np.allclose(m_expected, m_unitary), np.allclose(m_expected, m_matrix)
Out[7]: (False, True)

Proposal

Maybe we should store an explicit QubitOrder as well to improve consistency between the operator and underlying gate's representations?
Update: Note that QubitOrder is not serializable and hence it might make more sense to use an OrderedDict for qubit_pauli_map or an explicit Iterable[raw_types.Qid] for storing the qubit_order.

Upd: This is part of organizing gate relationships #3242

[shivanth]

Adding QubitOrder for Paulistring initialiser is possible.

1. This variable will have to be updated each time an operation (* in the above case) is performed on the gateOperation.
2. This will also need refactor to *mul*/*multiply* helpers in PauliString.
3. Use QubitOrder to return the property qubits

[tanujkhattar]

This should be done, let's figure out how to do it.