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

Reduce average gate fidelity to process fidelity #1788

Merged

Conversation

fhopfmueller
Copy link
Contributor

This PR reduces the calculation of average gate fidelity between two quantum channels to the recently merged implementation of process fidelity.

Additional minor change: raise an error when the channel passed to process_fidelity does not preserve the Hilbert space dimension of the input state. Previously it gave inconsistent results for that case, for different representations of equivalent channels. It's probably possible to define the process_fidelity and average_gate_fidelity for non-dimension-preserving channels if there is a use case for it? The previous version of average_gate_fidelity also was not defined for non-dimension-preserving channel.

Related issues or PRs
Last part of the changes proposed in #1703. Builds on the implementation of process_fidelity from #1712, #1748.

Changelog
Re-implement average_gate_fidelity using process_fidelity.

@hodgestar hodgestar added this to the QuTiP 5.0 milestone Mar 2, 2022
@hodgestar
Copy link
Contributor

@fhopfmueller If you can merge dev.major into this branch and resolve the conflicts, I can review (I think the conflicts are just with the fixes that you made that got merged into 4.7).

@hodgestar
Copy link
Contributor

@fhopfmueller I merged dev.major into your branch, but I ended up dropping most of the avg_gate_fidelity_test since the avg_gate_fidelity_test decorator didn't seem to be included in the PR commits. Do you know what happened to them? Should I add them back? And if so, where is the decorator implemented?

@coveralls
Copy link

coveralls commented Jul 1, 2022

Coverage Status

Coverage increased (+0.08%) to 70.898% when pulling 0cb3cdb on fhopfmueller:dev.major-agf-from-process-fid into 2cbd8db on qutip:dev.major.

@fhopfmueller
Copy link
Contributor Author

Thanks a lot for awakening this PR from its slumber @hodgestar !

All the decorator avg_gate_fidelity_test does is to skip the decorated test on Mac. Looks like it has been removed in the following commit which is included in your merge, in favour of refactoring the AGF tests into a class and skipping the whole class on Mac: 99ce2d5#diff-a9cd5a7620c34d60ffb61360a78bab270214a212c997e0c2cf5a6617443a90ab
(The decorator was originally introduced in #1034. The underlying issue #963 seems to be resolved, so maybe skipping the tests on Mac isn't necessary any more?)

The only test that is included in my original PR, but not after your merge, is test_average_gate_fidelity_against_legacy_implementation at https://github.com/fhopfmueller/qutip/blob/74080688cbece61a3d095fa6fc9dd347114b6152/qutip/tests/core/test_metrics.py#L443

Would you like to include that test? On the one hand, it contains copy-pasted code from the previous implementation, so the test isn't exactly elegant. On the other hand, it ensures that the new implementation of this PR gives the same result as the previous correct implementation. I'd favor including it.

@hodgestar
Copy link
Contributor

@fhopfmueller Thanks for the help! I've added back the test. I also made a separate commit to remove skipping the tests on Mac OS. Let's see what happens.

@hodgestar
Copy link
Contributor

Tests all pass on Mac OS now & the changes look good. Merging. Thanks @fhopfmueller.

@hodgestar hodgestar merged commit acaba9b into qutip:dev.major Jul 2, 2022
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

Successfully merging this pull request may close these issues.

None yet

3 participants