quantumlib
Should cirq.measure allow sets as inputs?
Is your design idea/issue related to a use case or problem? Please describe.
I was running some cirq Circuits and got confusingly unrepeatable results, until I realized that the problem was that I was doing something like
circuit = cirq.Circuit(input_circuit, cirq.measure(input_circuit.all_qubits()))
all_qubits produces a set, so constructing this kind of Circuit and sampling from it repeatedly can give inconsistent results since the order of the measured qubits may be different.
Describe your design idea/issue
Perhaps we should disallow passing Set or FrozenSet as the input to cirq.measure?
From cirq sync: Doug: We should add a big red flag to the documentation to call out that all_qubits returns set which doesn't preserve insertion order.
Pavol: Maybe we can change the ordering of unordered sequences (like sets) in the implementation of cirq.measure. If the input argument is ordered, then we can keep it as it is.
On the core question of whether cirq.measure should not accept sets, I don't think we should do that. There's no way in general to detect whether a given iterable comes from an "ordered" or "unordered" collection, so at best we'd be adding special cases for types that people happened to have problems with.
On all_qubits, it does seem unfortunate that AbstractCircuit.all_qubits iterates over the qubits in deterministic order for a given circuit only to then throw away that ordering information when we wrap the result in frozenset. We might consider changing this method to return an ordered set instead. Internally, we have OrderedSet and OrderedFrozenSet classes that would be easy to externalize if we wanted to do this. The implementation is very simple using a dictionary with empty values, since dict keys in python are an ordered set (it's an odd choice that stdlib sets are not ordered while dicts are).
I think one other drawback of all_qubits is that it iterates over the entire space time volume of the circuit (all moments x all qubits in moments) to figure out the qubit set. This is usually expensive and can hurt users if they are not careful (eg: using circuit.all_qubits() inside a loop instead of caching it once outside the loop).
We can also consider maintaining an internal dictionary of qubit_counts: Dict[cirq.Qid, int] that tracks the number of operations on each qubit. The dictionary can be easily updated when inserting / deleting operations in the circuit and then all_qubits() would just return the keys of this dictionary, which would also preserve insertion ordering (assuming we change the return type form frozen set to an ordered set or a tuple).
Yes, the expensive computation is an issue. I worry that trying to track properties such as all qubits in a circuit as it's being mutated is going to be very error-prone, and there are many other properties we'd potentially like to track in addition to the qubits so it'd be hard to cover everything. I'd prefer if we could encourage using immutable circuits with more explicit "builder" patterns when constructing or mutating circuits, since properties like all qubits can easily be cached on immutable objects (we already do this for FrozenCircuit.all_qubits). But it's going to be hard to push things in the direction of immutability because the mutable cirq.Circuit is so heavily used everywhere (and it's name is easier to type than cirq.FrozenCircuit).