search

quantumlib / Cirq

A Python framework for creating, editing, and invoking Noisy Intermediate Scale Quantum (NISQ) circuits.
Apache License 2.0
4.24k stars 1.01k forks source link

Bugfix in `_strat_has_stabilizer_effect_from_decompose` #6467

Closed tanujkhattar closed 7 months ago

tanujkhattar commented 7 months ago

Fixes https://github.com/quantumlib/Qualtran/issues/665

@NoureldinYosri Do you remember why you added the following check? 

    qid_shape = qid_shape_protocol.qid_shape(val, default=None)
    if qid_shape is None or len(qid_shape) <= 3:
        return None
codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (6a40da5) 97.82% compared to head (fb3c062) 97.82%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6467 +/- ## ======================================= Coverage 97.82% 97.82% ======================================= Files 1115 1115 Lines 97448 97452 +4 ======================================= + Hits 95327 95331 +4 Misses 2121 2121 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

NoureldinYosri commented 7 months ago

to stop the recursion. if it has 3 or less qubits then the from_unitary strategy should have caught it

https://github.com/quantumlib/Cirq/blob/6a40da5fc8661c3c253849d3d47d8bf45bd6134c/cirq-core/cirq/protocols/has_stabilizer_effect_protocol.py#L78-L85

if it outputs the wrong result then the bug will be there not here.

is there an example of this producing the wrong result?

tanujkhattar commented 7 months ago

is there an example of this producing the wrong result?

The bug was in decompose_protocol.decompose_once(val). That works only when val knows the qubits it acts upon (so operations, moments, circuits) but fails for gates. _try_decompose_into_operations_and_qubits works in the general case. I've added a tests that was failing earlier but passes now

to stop the recursion

I think the check is redundant, because the control flow would reach the decomposition strategy only when it couldn't figure out the answer using the first 3 strategies and that would imply a scenario where, for example, the operation decomposes into non unitary operations (eg: measurements + cliffords) and therefore we should continue to check decomposition. An example in the test shows a scenario where _unitary_ is not defined but decomposition and _has_stabilizer_effect_ is.

tanujkhattar commented 7 months ago

Let's wait and not merge right now, I need to run some more tests