-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fidelity calculation for density matrices improvement #4819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
This looks like a reasonable request to me. @viathor What do you think? For completeness, the modified formula can be derived as follows:
|
I agree the request is reasonable. Ideally, the PR implementing this would include a unit test demonstrating improved numerical stability. It should fail for the current implementation and succeed for the new one. @alvinquantum, any of the cases you encountered where fidelity exceeded 1 in cirq and stayed <=1 in your code can easily become such a unit test. Also, it'd be nice to have some evaluation of the effect on performance, e.g. ipython's %timeit output posted here would work. @tanujkhattar Your derivation LGTM with a nit that |
I think this is a working example. Here cirq.fidelity gives around 1.09 while get_fidelity gives 1 with a 10^-5 tolerance. The code starts with a 10 qubit state, traces out the 10th system, and then calculates the fidelity of the reduced state with itself. The get_fidelity can also be improved since it tends to give values greater than 1, but from my experience those values have been greater by around 10^-5.
|
Thanks, @alvinquantum! This is helpful. Note: this is good basis for a unit test, but should use cirq gates rather than converting from qasm (because ideally the unit test shouldn't be sensitive to bugs in our qasm converter!). |
Hi Google Quantum Team, Is this issue still open? If, yes please tell me more about it. |
@anonymousr007 Yes, the issue is still open. The goal is to replace the current implementation of calculating fidelity of two density matrices with the improved implementation that has lower numerical error. Current implementation: Cirq/cirq-core/cirq/qis/measures.py Lines 239 to 242 in 782c14e
New implementation: def get_fidelity(rho1, rho2):
rho1_sqrt=scipy.linalg.sqrtm(rho1)
rho2_sqrt=scipy.linalg.sqrtm(rho2)
return scipy.linalg.svdvals(rho1_sqrt @ rho2_sqrt).sum()**2 The PR implementing the change should also add a unit test demonstrating improved numerical stability of the new implementation, s.t. the current implementation fails but the new one passes. |
Please, assign this issue to me. I want to work on this. |
Marking as after 1.0 as this is a feature addition. |
Cirq/cirq-core/cirq/qis/measures.py
Lines 239 to 242 in 782c14e
The current implementation uses the direct definition of fidelity, which works well for small dimensions. From my experience, I was getting values significantly greater than 1. For the fidelity between rho1 and rho2, I found that squaring the sum of the singular values of the product of square_root(rho1) and square_root(rho2) gives a more accurate calculation. This equation is equal to fidelity. The idea is not mine and came from qutip.
This is my version of the implementation that I use.
The text was updated successfully, but these errors were encountered: