Update rg_qft_multiplier.py for Allowance of Float Exponentiation

[NahidaNahida]

As I have created an open bug report, there exists accidental trouble when processing scientific computing like exponentiation with values deriving from numpy. Actually, they are reasonable candidates as the inputs of this API, regardless of the application of numpy. For an occurrence, when I applied RGQFTMultiplier(n, m) with $n=2$ and $m=2$, nothing unexpected happened. By contrast, if the variables $n$ and $m$ were generated from sentences like np.arange(1,5) and np.arange(n, 2 * n + 1), the program would terminate out of my expectation, leaving a warning that Integers to negative integer powers are not allowed.

Summary

To cope with it, I modify the int type 2 to the float type 2.0. Then, this API can run in a normal manner, which seems to accept inputs in the numpy form, like np.int64(2).

Comment

In my opinion, this kind of bug may not merely occur in this API, and quantum program developpers may trigger the same warning as accidentally using numpy package, where it is not attributed to their mistakes. It is still meaningful to seek out the latent bugs in other APIs upon qiskit, because compared to other programs, quantum programs require quite a large scale of algebraic operations based on Python language.

[qiskit-bot]

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

• @Cryoris
• @Qiskit/terra-core
• @ajavadia

[CLAassistant]

All committers have signed the CLA.

[Cryoris]

Thanks for discovering and taking this issue on! Though we have to be careful to correctly implement the adder. In the referenced paper (see the docstring), the authors state

Therefore it seems we would have to make a case distinction if the power is negative instead of simply allowing it. Could you add a test for this case to ensure the result is correct?

[NahidaNahida]

Thanks for discovering and taking this issue on! Though we have to be careful to correctly implement the adder. In the referenced paper (see the docstring), the authors state

Therefore it seems we would have to make a case distinction if the power is negative instead of simply allowing it. Could you add a test for this case to ensure the result is correct?

Thanks for your further advice @Cryoris . It is exciting and amazing for a test researcher to identify this kind of latent bug I have never noticed. As an initial attempt, I've referred to the source codes and discovered in surprise that there is no logical condition to distinguish the consequence whether $j+s-n>0$ or not. There are the original code lines in qiskit.circuit.library.RGQFTMultiplier.

for j in range(1, num_state_qubits + 1):
    for i in range(1, num_state_qubits + 1):
        for k in range(1, self.num_result_qubits):    
            lam = (2 * np.pi) / (2.0 ** (i + j + k - 2 * num_state_qubits))   
            circuit.append(
                PhaseGate(lam).control(2),
                [qr_a[num_state_qubits - j], qr_b[num_state_qubits - i], qr_out[k - 1]],
            )

Based on the reference provided, I have come to understand the function that the codes are designed to achieve. Thus, with the help of differential testing, I have added an if sentence to these codes as follows. As we know, just before applying the controlled gate, this additional conditional logic would not pollute the corrected program version.

for j in range(1, num_state_qubits + 1):
    for i in range(1, num_state_qubits + 1):
        for k in range(1, self.num_result_qubits):     
            if i + j + k - 2 * num_state_qubits > 0:            # added sentence
                lam = (2 * np.pi) / (2.0 ** (i + j + k - 2 * num_state_qubits))   
                circuit.append(
                    PhaseGate(lam).control(2),
                    [qr_a[num_state_qubits - j], qr_b[num_state_qubits - i], qr_out[k - 1]],
                )

For the testing phase, I have tried to utilize swap test to compare the final states from the above two versions. If all the measurement results from swap test are $0$ (I set to measure for $10$ times), it will be identified to pass, otherwise fail. Then, I simply generate test cases by combining possible ranges of input variables, that num_state_qubits $n\in [1,3]$, num_result_qubits $m\in [n,2n]$ and the computational states as input quantum states $|x\rangle, x\in[0,2^n-1]$. By testing with $312$ test cases, $122$ of them pass while $190$ of them fail. In conclusion, I think something wrong still remains in the program. If there is something wrong with my testing, please connect with me.

[jlapeyre]

Thanks for pursuing this bug. This PR will also need

• A release note. This should wait until the nature of the fix is sorted out.
• Tests.

In addition to what you and @Cryoris investigate in the comments above, there is the following.
There are alredy some tests for RGQFTMultiplier. If I wrap the integer parameters used in these tests with np.int64() the tests fail due to the bug you uncovered. I suggest adding a loop, or more data to repeat the current tests for type np.int64 and np.int32. Again, this should wait until we find whether we need to correct the algorithm and the existing tests.

[NahidaNahida]

Well, I am glad to wait for your conclusion @jlapeyre. In fact, I think testing quantum programs varies from testing classical programs, especially for the quantum circuits by unit testing. Apart from paying attention to possible different types of bit inputs, we should take the appropriate initial quantum states into account. However, simple test cases input through the ports of API may not include or cover the input Hilbert space of quantum states. The way I try to test with various quantum states is to construct another quantum circuit to prepare these states and then append the circuit to be tested. It seems to be indirect and the test oracle (direct measurement for immense times or measure for fewer times by swap test) is worth to be clearified 🤔.