circuit.library.graph_state needs testing

[1ucian0]

The object GraphState (in qiskit/circuit/library/graph_state.py) needs testing. No line is currently coveraged.

[conorp854]

Hi, I'd be interested in contributing if this issue is still available. This would be my first issue so I'm not completely sure where to start!

[mantcep]

Hi, I would like to work on this issue, would it be possible to assign this to me if it is not being worked on?

[Cryoris]

Hi!

Great that you want to contribute 🎉

So the task would be adding tests for the GraphState circuit. This concept is introduced in this paper which you can look up for some context.
A test should make sure that the implementation in Qiskit works (i.e. it runs w/o errors) and is correct (i.e. the output is correct).

Ideally for such a circuit we would check the implementation on a theoretical level. This means we're checking the Qiskit implementation against what the maths says. This can e.g. be done by computing the unitary the circuit implements and then simulating the circuit in Qiskit.
This is done e.g. for the HiddenLinearFunction:
https://github.com/Qiskit/qiskit-terra/blob/8771e595a67e584f7ce1bac2d70db84516cd3432/test/python/circuit/test_library.py#L150
and the QFT:
https://github.com/Qiskit/qiskit-terra/blob/8771e595a67e584f7ce1bac2d70db84516cd3432/test/python/circuit/test_library.py#L346
To do this you could have a look at Eq. (9) and following in the paper, where it describes what state the circuit builds (called |G> in the paper). For this check we would compute the matrix that prepares |G> as described in the paper and compare it to the GraphState.

Another simpler way, which would already be a good start, is to compare the circuit to a hardcoded reference circuit. This is e.g. done for the IQP circuit:
https://github.com/Qiskit/qiskit-terra/blob/8771e595a67e584f7ce1bac2d70db84516cd3432/test/python/circuit/test_library.py#L190

You can check this reference files and let me know if you have any questions! What's the tactic you would like to pursue?

[mantcep]

Hi @Cryoris!

Thank you so much for going into such details and explaining this so clearly! Your description has provided more than enough background for me to work on this 🙂

I would like to make a check on a theoretical level, it seems quite straightforward to calculate the matrix by hand for a particular graph. Would it be fine if I pick a smaller graph that can be implemented by up to 4 qubits to be used for testing, e.g., the graph No. 3 in FIG. 4 of the paper? I am also considering creating multiple tests for graphs of various sizes, but let me know if that would be too much.

Thank you again for guidance!

[mantcep]

Hi again!

I just wanted to give a quick update about this issue. I have looked into testing the graph states a bit more and I have come up with a piece of code that creates the expected unitary for a given adjacency matrix. I thought this might be a relevant update, because my previous point from the above comment that I would be calculating the matrix by hand is not valid anymore. I think it makes more sense to numerically calculate the expected matrix based on the adjacency matrix, since that would allow to change the test by simply changing the adjacency matrix. I can see that this is the approach used in other test cases, e.g., for HiddenLinearFunction. This was most likely the approach that was expected to begin with, so I am sorry to have figured that out just now 🙂

As a quick outline, the way I calculate the expected matrix from the adjacency matrix is quite simple. First, I create the matrix for Hadamard gates for the number of qubits extracted from the adjacency matrix. Secondly, I create the matrices for the CZ gates based on the adjacency matrix. This step is slightly more involved, since I need to get the CZ gate for any given number of qubits and control-target connections. I do that by looping through an identity matrix and setting the value of the diagonal elements to -1 where both the control and target are in state |1>. Lastly, all the matrices are multiplied together to get the expected unitary. I have tested this approach and the results look good.

I would now like to work on adding the mentioned code to test/python/circuit/test_library.py. Once the code is properly implemented and I am satisfied with the addition, I would like to create a pull request, if that would be fine 🙂

[Cryoris]

Hi @mantcep,

sounds great that you found a way to compute the generic unitary transformation that creates the graph state. A fixed comparison would also be good, but if we can do it generically that's of course better.

It is easier to comment on the code if we can see it, so feel free to open a pull request already now and name it [WIP] Your title to show that it is still work in progress. Instead of looping through all indices you might also be able to compute the indices of control and target qubit and directly check their states. But I'll give more feedback once you opened the PR!

Sounds like a good plan 👍

[mantcep]

Hi @Cryoris,

I have created a pull request as suggested. I tried looking into a better way of getting the CZ gate, but I could not think of anything straightforward. Let me know what you think, I could put more time into vectorizing the loop over the identity matrix.

Thank you for your time! 🙂