Fixes for device wire handling #42
Conversation
| self._unsorted_qubits = qubits | ||
| self.qubits = sorted(qubits) | ||
|
|
||
| super().__init__(wires, shots, analytic) |
There was a problem hiding this comment.
I don't understand why super is not called earlier? That would remove a lot of code above, because num_wires can be extracted from len(self.wires) or self.num_wires. One could still raise an error if qubits has a different length.
There was a problem hiding this comment.
@mariaschuld, here is @co9olguy's original comment: #40 (comment)
I had to move down the superclass initialization here, since it is the thing that creates the wire_map, but the wire_map can't be determined until the cirq qubits have been assigned
There was a problem hiding this comment.
Ah maybe that is due to other changes made in the original PR? In that case we can merge as is, to not to create conflicts!
There was a problem hiding this comment.
define_wire_map is called in the parent class's __init__ method.
We don't need to know the number of qubits (as you say, that can be inferred), but we do need to do all this qubit pre-processing before define_wire_map makes sense to be called
| consecutive_wires = Wires(cirq_order) | ||
|
|
||
| wire_map = zip(wires, consecutive_wires) | ||
| return OrderedDict(wire_map) |
There was a problem hiding this comment.
Worked really nicely once I figured out that Cirq always uses an ordering internally (the edge case of unordered qubits was never caught before in this plugin)
| # wires that are not acted upon | ||
| self.circuit.append(cirq.IdentityGate(len(self.qubits))(*self.qubits)) | ||
| for q in self.qubits: | ||
| self.circuit.append(cirq.IdentityGate(1)(q)) |
There was a problem hiding this comment.
Not sure I understand why len(self.qubits) can be replaced by 1?
There was a problem hiding this comment.
In this case, it looks like you are applying len(self.qubit) single-qubit identity gates, instead of one large identity gate.
There was a problem hiding this comment.
Yes, but this is just putting a band-aid over an existing hack. Would be better for us to keep better track of the unused wires in the plugin (which we can now do easily!)
There was a problem hiding this comment.
Have added a TODO in the codebase
|
Thanks @mariaschuld! I'll get @co9olguy to give it a once over before we merge it in. |
This PR contains the pasqal-independent bug fixes from #40 by @co9olguy