-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add an assert method testing the equality of two counts #211
base: main
Are you sure you want to change the base?
Conversation
@@ -67,6 +67,9 @@ def test_rz(self,): | |||
# qiskit.mcrz is multi-controlled Z rotation up to a global phase, | |||
# which means that qiskit.mcrz coincides with multi-controlled p gate. | |||
# (Namely, qiskit.mcrz is same as qiskit.mcp.) | |||
@unittest.skip( | |||
"This test fails due most likely to a version update of qiskit." | |||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is skipped since it fails due most likely to a version update of Qiskit:
======================================================================
FAIL: test_mcrz (simulator.state_vector_circuit.test_rz_gate.TestStateVectorCircuitRyGate)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/runner/work/quantestpy/quantestpy/test/with_qiskit/simulator/state_vector_circuit/test_rz_gate.py", line 75, in test_mcrz
qpc = cvt_input_circuit_to_quantestpy_circuit(qc)
File "/home/runner/work/quantestpy/quantestpy/quantestpy/converter/converter_to_quantestpy_circuit.py", line 23, in cvt_input_circuit_to_quantestpy_circuit
quantestpy_circuit = _cvt_qiskit_to_quantestpy_circuit(circuit)
File "/home/runner/work/quantestpy/quantestpy/quantestpy/converter/sdk/qiskit.py", line 365, in _cvt_qiskit_to_quantestpy_circuit
raise QuantestPyError(
quantestpy.exceptions.QuantestPyError: Qiskit gate [unitary] is not supported in QuantestPy.
Implemented qiskit gates: ['ccx', 'ccz', 'ch', 'cp', 'crx', 'cry', 'crz', 'cs', 'csdg', 'cswap', 'csx', 'cu', 'cx', 'cy', 'cz', 'h', 'id', 'iswap', 'mcp', 'mcx', 'p', 'r', 'rx', 'ry', 'rz', 's', 'sdg', 'swap', 'sx', 'sxdg', 't', 'tdg', 'u', 'x', 'y', 'z']
----------------------------------------------------------------------
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@junnaka51
Thank you for your contribution!
But I have a question about the definition of "tole".
Please check my inline comments.
) | ||
) | ||
|
||
def test_approx_equivalent(self,): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did not test_approx_equivalent
give an error?
I think your definition of "tole" gives an error when key = "00":
abs(100-80) > sqrt(100) + sqrt(80)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because the default value to sigma
is 2
, and the following is satisfied:
abs(100-80) < 2 * (sqrt(100) + sqrt(80))
err_b = np.sqrt(v_b) | ||
|
||
diff = np.abs(v_a - v_b) | ||
tole = (err_a + err_b) * sigma |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you define it this way?
I think this definition is unsuitable because the dimension of 'diff' is different from that of 'tole'.
Of course, count
has no dimension in this case, but please image the case when the dimension of v_a
is length(cm
).
If there are a lot of data, we may use Root Sum Squire
as the definition of 'tole'.
But I don't know an appropriate definition when we compare two values...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is assumed that counts_a[key]
and counts_b[key]
are always the number of times each bitstring key
was measured. I believe that users can understand it from the document doc/assertion/assert_equivalent_counts.md
.
The standard deviation for the number of times measured N
is sqrt(N)
, therefore the definition of tole
should be okay. Another possibility may be
tole = sqrt(v_a + v_b) * sigma
It is fine for me to define the both and leave users to decide which one they use?
Summary
This PR intends to add a new assert method
assert_equivalent_counts(counts_a, counts_b)
testing the equality of the two counts.Example