GroenteLepel / qiskit-quantum-knn

A pure quantum knn classifier for a gated quantum computer.
Apache License 2.0
22 stars 8 forks source link

Possible error in mapping states to integers #11

Open knodelgino opened 3 years ago

knodelgino commented 3 years ago

When computing fidelities/contrasts, there is a step where we map the states of the computational basis (e.g. '10010101') back to integers using

index_state = int(comp_state, 2)

However, I read in the documentation of Qiskit that the ordering they use for tensor product states is the opposite of most physics textbooks:

When representing the state of a multi-qubit system, the tensor order used in Qiskit is different than that used in most physics textbooks. Suppose there are 𝑛 qubits, and qubit 𝑗 is labeled as 𝑄𝑗. Qiskit uses an ordering in which the 𝑛th qubit is on the left side of the tensor product, so that the basis vectors are labeled as π‘„π‘›βˆ’1βŠ—β‹―βŠ—π‘„1βŠ—π‘„0.

For example, if qubit zero is in state 0, qubit 1 is in state 0, and qubit 2 is in state 1, Qiskit would represent this state as |100⟩, whereas many physics textbooks would represent it as |001⟩.

I think this means we might have to change the code to

index_state = int(comp_state[::-1], 2)

I played around with this a little bit in a juptyer notebook, where I tried to explicitly compute the fidelities of some sample states, and found that without this change, a lot of them get swapped around. Reversing the order as above seems to fix the issue.

If what I'm saying is correct, we'd have to make 2 changes:

https://github.com/GroenteLepel/qiskit-quantum-knn/blob/2b382089f8ffdd0301dca09fb5807efdddc19fa0/qiskit_quantum_knn/qknn/qkneighborsclassifier.py#L366

https://github.com/GroenteLepel/qiskit-quantum-knn/blob/2b382089f8ffdd0301dca09fb5807efdddc19fa0/qiskit_quantum_knn/qknn/qkneighborsclassifier.py#L441

knodelgino commented 3 years ago

@GroenteLepel

GroenteLepel commented 3 years ago

Thank you very much for this contribution. I must admit: it has been a while since I have taken a look at this code. I do remember however Qiskit’s reversed order of qubits and having to fiddle around with that.

I appreciate your thoughts on this, and I would suggest you create a merge request of your solution so that I can merge it!

knodelgino commented 3 years ago

awesome, I will look into it.

knodelgino commented 3 years ago

@GroenteLepel created a PR here: https://github.com/GroenteLepel/qiskit-quantum-knn/pull/12