Skip to content
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

Suggested improvements for average gate fidelity and process fidelity #1703

Closed
fhopfmueller opened this issue Nov 1, 2021 · 3 comments
Closed

Comments

@fhopfmueller
Copy link
Contributor

Hello,

I have some suggestions for improvements in some functions in metrics.py. I'd be happy to put together a PR.

The current implementation of average_gate_fidelity is based on the Kraus representation of the input quantum channel. If the input is a superoperator, it is first converted to its Kraus representation (involving diagonalization) before computing the average gate fidelity, which sacrifices performance and accuracy. There is a direct formula starting from a superoperator, see, e.g., https://qiskit.org/documentation/stubs/qiskit.quantum_info.average_gate_fidelity.html relating the average gate fidelity to the process fidelity, and https://qiskit.org/documentation/stubs/qiskit.quantum_info.process_fidelity.html#qiskit.quantum_info.process_fidelity to compute that from a superoperator. I think it would be better to use that direct formula if the input is not already in Kraus form!

There is also a function process_fidelity in Qutip, but I'm having a hard time understanding what it's intended for. It doesn't seem to be the process fidelity explained in the Qiskit docs above. The arguments of Qutip's process_fidelity are called U1 and U2 suggesting they are expected to be unitary, but this is not checked. It is computed as (U1 * U2).tr(), which is not between 0 and 1, and doesn't give 1 as I'd expect if U1==U2. Does someone know the intended use of that function? If not, I'd implement the process fidelity as described in the Qiskit docs here, with several version depending on whether the input is Kraus, a superoperator, and maybe a chi matrix.

Lastly, there is an issue with the tests - the lines

class Test_dnorm:
    # Skip dnorm tests if we don't have cvxpy or cvxopt available, since it
    # depends on them.
    cvxpy = pytest.importorskip("cvxpy")
    cvxopt = pytest.importorskip("cvxopt")

actually skip all the tests in the file if cvxpy is not available, not just the ones in the Test_dnorm class. The test output, if cvxpy is not installed, is

$ pytest qutip/tests/test_metrics.py 
============================= test session starts ==============================
platform linux -- Python 3.9.7, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /home/florian/code/qutip/qutip/qutip/tests, configfile: pytest.ini
plugins: rerunfailures-10.1
collected 0 items / 1 skipped  

That issue seems to be addressed on the dev.major branch. Would it make sense to take the test file from there and use it in the master branch also?

Thanks!

@hodgestar
Copy link
Contributor

@fhopfmueller Thank you for looking into these various issues! PRs would be very much appreciated. Perhaps lets do separate PRs for the tests, then process_fidelity and then average_gate_fidelity?

I would like to release 4.7 in the not too distance future, so one option is to target dev.major / 5 directly (in which case the tests are already fixed).

@fhopfmueller
Copy link
Contributor Author

Hi @hodgestar , sounds good! I submitted the first PR for the tests.
For the process_fidelity and average_gate_fidelity PRs, I can definitely send them on the weekend, I can't promise before that unfortunately.
I'm happy to target either version with the other PRs, please let me know what makes more sense!

@fhopfmueller
Copy link
Contributor Author

Closing this since the last PR has been merged. Thanks for your advice and help, @Ericgig @hodgestar !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants