Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
QCmp.qubit_comparatorreturns inverted results forfunction='eq'andfunction='neq'wheneverq_state_1has more qubits thanq_state_2. Thefour inequality modes are not affected by this bug; only the two symmetric
modes break, and only when the first register is the wider one.
Repro,
q_state_1in uniform superposition over 0..7 andq_state_2holding 2,once with a 3-qubit second register and once with a 2-qubit one:
Padding
q_state_2out to the width ofq_state_1gives the right answer, sothe two columns should agree.
eqandneqare exactly swapped.Root cause
pyqpanda-algorithm/pyqpanda_alg/QCmp/QCmp.py,qubit_comparator, lines357-360 before this change.
The function normalises the two registers so that the shorter one is the one it
walks over. When
len(q_state_1) > len(q_state_2)it swaps them andcompensates with a negation:
Swapping the operands of a comparison turns
a > bintob > a, which is thecomplement of
a >= b, so the negation plus theflagreassignment on the nextlines is what keeps
g,geq,sandseqcorrect. Buteqandneqaresymmetric:
a == bandb == aare the same predicate, so nothing needscompensating and the
X(q_cmp)is a straight inversion of the answer. Theswap branch applies it unconditionally.
Fix
Skip the negation for the two symmetric modes:
Three lines, no gate-count change for any mode that was already correct, and one
gate fewer for
eq/neqon the swap path. The non-swap branch is untouched, soevery equal-width and
n1 < n2call produces an identical circuit tobefore, including the docstring example and
demo14-comparator-qubit_comparator.ipynb(both equal-width,
function='g').Verification
Exhaustive computational-basis sweep, asserting the comparison qubit against the
classical predicate for every
(a, b)pair, over 6 modes and 7 register shapeswith both widths at least 2 (2400 pairs in total):
Before the fix: 1952/2400 pass, and the 448 failures are exactly the six
bolded cells, i.e. every
eq/neqcase withn1 > n2fails, none of thempartially. After the fix: 2400/2400 pass, every cell full.
Out of scope here: when either register is a single qubit and the widths differ
(shapes like (1,2) or (3,1)), all six modes already return some wrong flags on
develop, because the loop over the narrow register never consumes the extrahigh bits of the wide one. That is a separate defect with a different cause. This
change does not touch it, and the sweep above uses widths of 2 and up.
Tests
test/QAlgBase/Test_comparator_qubit_comparator.pyexisted but every line wascommented out, so nothing covered this function. Un-commented it, brought it up
to the current pyqpanda3 API, and extended it to 50 tests: the docstring
example, all six modes on the superposition case with exact expected
probabilities, the
a == bboundary at equal and unequal width, and exhaustivebasis-pair sweeps at shapes (2,2), (3,3), (3,2), (2,3) and (4,2) for all six
modes, plus the
NameErroron an unknownfunction.Against the unpatched code the new module reports 6 failed, 44 passed; the six
failures are the
eqandneqparametrisations of the three first-register-widertests. Against the patched code all 50 pass.
One note on the test module: pyqpanda3 raises
qubits_list arg errorfromget_prob_dictwhen a qubit in the program was never acted on, which happens onthe short circuits some of these modes build, so the helper applies an
Itoevery qubit in the register before measuring.
#42 also touches this file but a different function (
qft_qubit_comparator),and the two branches merge cleanly with no conflict.