[Bugfix] Tensor representation of controlled operations with qubit support order #40
Conversation
| @@ -61,11 +62,41 @@ def tree_unflatten(cls, aux_data: Any, children: Any) -> Any: | |||
| return cls(*children, *aux_data) | |||
|
|
|||
| def unitary(self, values: dict[str, float] = dict()) -> Array: | |||
There was a problem hiding this comment.
It doesn't seem like values is used anywhere in this method unless I'm wrong ?
There was a problem hiding this comment.
Indeed, it is mostly picked up later by Parametric. We need to revisit these classes at some point.
| return _dagger(self.unitary(values)) | ||
|
|
||
| def tensor(self, values: dict[str, float] = dict()) -> Array: |
There was a problem hiding this comment.
Hm, it feels like the semantics between unitary and tensor are close enough to be merged ? Is there a need for two methods ? It feels a bit like an overkill though.
There was a problem hiding this comment.
Yes it is a bit ill-defined but basically, in the apply functions, a different _controlled function is used that avoid permuting according to the qubit order, as it is not required to apply an operator. However, .tensor is required for Qadence, and especially for block_to_tensor.
| target_qubits: TargetQubits, | ||
| control_qubits: ControlQubits, |
There was a problem hiding this comment.
Is there a need for two types here ?
There was a problem hiding this comment.
It is legacy. Want to remove it?
| if isinstance(control_qubits[0], tuple): | ||
| controls = control_qubits[0] | ||
| if isinstance(target_qubits[0], tuple): |
There was a problem hiding this comment.
Does it mean that control_qubits and target_qubits could be tuples of tuples ?
There was a problem hiding this comment.
Yes, legacy again for being able to sum them for the reduce operator a previous person wrote before me.
| control_ind_support = tuple(i for i, q in enumerate(qubit_support) if q in controls) | ||
|
|
||
| # Create the full Hilbert space dimension | ||
| full_dim = 2**ntotal_qubits |
There was a problem hiding this comment.
| full_dim = 2**ntotal_qubits | |
| full_dim = 2 ** ntotal_qubits |
There was a problem hiding this comment.
lint does not agree with you :)
Resolves #35