Skip to content

Cross entropy benchmarking functionality does not match documentation #2757

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

Closed
kevinsung opened this issue Feb 10, 2020 · 9 comments · Fixed by #4853
Closed

Cross entropy benchmarking functionality does not match documentation #2757

kevinsung opened this issue Feb 10, 2020 · 9 comments · Fixed by #4853
Assignees
Labels
area/docs area/experiments area/xeb kind/bug-report Something doesn't seem to work. status/stale This has been closed due to inactivity for an extended period of time. triage/wont-fix Decided against going forward with this

Comments

@kevinsung
Copy link
Collaborator

kevinsung commented Feb 10, 2020

Documentation:

$f_{mn}^{meas} = \langle D * P^{th, mn}_q - 1 \rangle$

Code:
pp_cross = probs_exp * probs_meas
pp_exp = probs_exp**2
f_meas = np.mean(num_states * np.sum(pp_cross, axis=1) - 1.0)

In the code, f_meas seems to be a quite different thing from what's documented: it is obtained by estimating the experimental bitstring probabilities prob_meas through a experimental histogram (note that most of these will be 0 at a modest number of qubits), and then multiplying that number by the theoretical probability.

It appears that the documentation is based on the assumption of exponentially distributed (Porter-Thomas) probabilities, while the code is doing something else. Is that right?

@XiaoMiQC

@dabacon
Copy link
Collaborator

dabacon commented May 8, 2020

@viathor also to comment

@balopat balopat added kind/bug-report Something doesn't seem to work. kind/docs area/experiments area/xeb triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Sep 21, 2020
@mpharrigan
Copy link
Collaborator

@viathor @XiaoMiQC is this an issue?

@XiaoMiQC
Copy link
Contributor

I believe the formula in the documentation is actually equivalent to what is being done in the code. This has been used on actual quantum processors and I have obtained fidelity numbers as expected. One can test the equivalence by generating bits using the Cirq simulator and compute the results using the code. If it is different than what you get out of implementing the formula in the documentation, please let me know.

@dstrain115 dstrain115 added triage/wont-fix Decided against going forward with this and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Oct 6, 2021
@dstrain115
Copy link
Collaborator

Circ cync: Looks like this is correct actually, @viathor will doublecheck and update status.

@viathor
Copy link
Collaborator

viathor commented Oct 16, 2021

@XiaoMiQC @kevinsung @dstrain115

Kevin is right. Docstring and code disagree. The former describes the linear cross-entropy benchmarking (which was our ultimate choice in QS experiment back in 2019). The fidelity estimate in this method is produced by averaging the expression 2^n p_th - 1 over the experimentally observed bitstrings (where p_th stands for the ideal/theoretical probability of each bitstring).

OTOH, the code appears to use Vadim et al's Least Mean Square for XEB wherein the fidelity estimate is computed as the ratio (dH_measured * dH_ideal) / dH_ideal**2. Both are known to work, but produce slightly different estimates with different statistical properties (IIRC, the former has better variance).

I don't think it makes sense to change the code, but we should update the docstring.


Also, it is worth noting that a number of XEB-based estimators of fidelity are implemented in fidelity_estimation.py. In particular, see linear_xeb_fidelity_from_probabilities and least_squares_xeb_fidelity_from_probabilities which implement the two estimators referred to above. In addition, the module implements log XEB and HOG score.

Thus, some of the XEB functionality is unfortunately duplicated. What do y'all think about removing cross_entropy_benchmarking.py? Does anyone still use this code? Could they switch to the newer and more versatile alternative in fidelity_estimation.py and related(*) modules?

(*) Note that fidelity_estimation.py has a narrower focus than cross_entropy_benchmarking.py. For example, it does not have code to generate RQCs. That said, AFAIK code to do all other XEB-related tasks exists elsewhere in cirq.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days

@github-actions
Copy link

Issue closed due to inactivity.

@mpharrigan mpharrigan reopened this Dec 15, 2021
@mpharrigan
Copy link
Collaborator

for your second comment @viathor there's #3775

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days

CirqBot pushed a commit that referenced this issue Jan 18, 2022
Closes #2757.

The "exp" suffix may be interpreted as either "experimental" or "expected" (as in "theoretically expected") which causes confusion that led to #2757. This PR changes the suffix to "th" (for "theoretical") which happens to be what is already used in the docstring. Also, added some missing docstrings.
MichaelBroughton pushed a commit to MichaelBroughton/Cirq that referenced this issue Jan 22, 2022
)

Closes quantumlib#2757.

The "exp" suffix may be interpreted as either "experimental" or "expected" (as in "theoretically expected") which causes confusion that led to quantumlib#2757. This PR changes the suffix to "th" (for "theoretical") which happens to be what is already used in the docstring. Also, added some missing docstrings.
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
)

Closes quantumlib#2757.

The "exp" suffix may be interpreted as either "experimental" or "expected" (as in "theoretically expected") which causes confusion that led to quantumlib#2757. This PR changes the suffix to "th" (for "theoretical") which happens to be what is already used in the docstring. Also, added some missing docstrings.
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
)

Closes quantumlib#2757.

The "exp" suffix may be interpreted as either "experimental" or "expected" (as in "theoretically expected") which causes confusion that led to quantumlib#2757. This PR changes the suffix to "th" (for "theoretical") which happens to be what is already used in the docstring. Also, added some missing docstrings.
@mhucka mhucka added T status/stale This has been closed due to inactivity for an extended period of time. labels Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs area/experiments area/xeb kind/bug-report Something doesn't seem to work. status/stale This has been closed due to inactivity for an extended period of time. triage/wont-fix Decided against going forward with this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants