Semiclassical Fourier Transform #688
Conversation
* only when opted in by user; when directly precedes measurement * currently only for not inverse, not do_swaps, and no approximation
|
|
|
|
|
First apologies for delay in response. Would you have any reference material or paper that explains this? If not can you supply any additional documentation within the code that explains this technique further? |
|
@woodsp-ibm No worries whatsoever! I completely forgot to link to the papers in the initial PR message:
Additionally, I found time to work out the inverse, and just committed that (along with a similar demo notebook, attached below). This should be just as useful because a good number of classic algorithms end with inverse QFT + measurement (e.g. phase estimation). I absolutely agree, both a unit test and a showcase tutorial would be very beneficial. Unfortunately I'm extremely pressed for time at the moment, so it might be a little while before I can get them together. |
There was a problem hiding this comment.
Hi @gillenhaalb, very nice addition to the Fourier transform!
It would be great to have at least one (ideally a few) tests for your modification, to check that it doesn't break anything. E.g. you could test an algorithm that uses Fourier transformation preceding measurements (such as AmplitudeEstimation), or compare the transform to the "usual" Fourier transform before a measurement.
Let me know if you need help with these tests!
| circuit.u1(lam, j + (k + 1)).c_if(circuit.cregs[j], 1) | ||
| return circuit | ||
| else: | ||
| inv_qubit_range = range(len(qubits) - 1, -1, -1) |
There was a problem hiding this comment.
Is inv_qubit_range necessary? It's equal to qubit_range, which is defined above (in line 94).
| else: | ||
| inv_qubit_range = range(len(qubits) - 1, -1, -1) | ||
| for _ in inv_qubit_range: | ||
| circuit.add_register(ClassicalRegister(1)) |
There was a problem hiding this comment.
This doesn't need a for loop, just write circuit.add_register(ClassicalRegister(len(qubits))).
| if precedes_measurement and not do_swaps and approximation_degree == 0: | ||
| if not inverse: | ||
| for j in qubit_range: | ||
| circuit.add_register(ClassicalRegister(1)) |
There was a problem hiding this comment.
Consider taking this outside the for loop for simplicity. Since this register is both needed for the inverse and normal QFT, you can also take this outside the if statement, i.e. before if not inverse you could put circuit.add_register(ClassicalRegister(len(qubits))).
| circuit.measure(j, j) | ||
| for qubit in neighbor_range: | ||
| k = j - qubit | ||
| lam = - np.pi / float(2 ** (k)) |
There was a problem hiding this comment.
You don't need a conversion to float here (and also no extra-brackets around k)
| circuit.h(j) | ||
| circuit.measure(j, j) | ||
| for k in neighbor_range: | ||
| lam = 1.0 * np.pi / float(2 ** (k + 1)) |
There was a problem hiding this comment.
You don't need to multiply with 1.0, nor a conversion to float here. Python 3 works does float arithmetics anyways (and on top you already use the float number np.pi).
|
@Cryoris Thanks for the review and comments! I totally agree, more unit tests (and some code tidying) are absolutely necessary before merging. Unfortunately we're in the middle of exam season at the moment, so I won't be able to put in more work on those for a while. However, I was thinking I would ultimately do phase estimation for the "algorithm test". In the meantime, you can check out the two zip files in my above comments (if you didn't already see them), which contain Jupyter Notebook demos comparing the semiclassical to the "usual" QFT. |
|
Maybe this could also be added as transpiler pass, that checks whether a QFT directly precedes a measurement and then uses this version instead of the normal one. However this would probably require the "abstract" QFT block to be present up to transpilation such that it is easily identified. |
|
@Cryoris Given that QFT circuit has been moved over to circuit library in Terra this PR no longer really applies to Aqua in its current form. Can we give some thought to progress this PR one way or another. |
|
Yes, this should go to Terra. I think this would fit well into the circuit library, from there the transpiler could also grab it and replace the usual QFT with this one if applied before a measurement (once we start to implement this feature) |
|
@Cryoris Is there some aspect of this that should be raised as a issue on Terra such that this functionality can be added there? Since QFTs are now part of circuit library in Terra this seems no longer relevant here and we should close it? |
|
As the QFTs have been moved and refactored in Terra this is no longer valid here, so as such I am closing it. |
Summary
Added option to make Quantum Fourier Transform "semiclassical" when it directly precedes measurement. This replaces two-qubit control gates with measurement and classical control, greatly reducing noise (see notebook attached for noise reduction demo).
Details and comments
SemiclassFTdemo.zip