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

Implement consistancy checks for unitary protocol in the presence of ancillas #6182

Closed
NoureldinYosri opened this issue Jul 5, 2023 · 7 comments
Assignees
Labels
good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. kind/feature-request Describes new functionality triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add

Comments

@NoureldinYosri
Copy link
Collaborator

NoureldinYosri commented Jul 5, 2023

Is your feature request related to a use case or problem? Please describe.
#6101 was created to update cirq.unitary and cirq.apply_unitaries protocols to support the case when gates allocate their own ancillas. This was achieved in #6112, however the fix assumes the decomposition is correct. a consistency check is need to check that. This is the fourth task on #6101 (comment).

The consistency check should check that the result is
1. Indeed a unitary
2. CleanQubits are restored to the $\ket{0}$ state.
3. Borrowable Qubits are restored to their original state.

Describe the solution you'd like
for the correctness checks we need a cirq.testing.assert_consistent_unitary that does the checks listed above.

What is the urgency from your perspective for this issue? Is it blocking important work?
for the first task
P1 - I need this no later than the next release (end of quarter)

for the second and third tasks
P3 - I'm not really blocked by it, it is an idea I'd like to discuss / suggestion based on principle

@shef4
Copy link
Contributor

shef4 commented Jul 5, 2023

@NoureldinYosri Thanks for breaking this down. I got my dev environment working and can start testing and adding code.

I know you said you'd work on task 1 in #6112, but I'd be happy to work on it if you've got other task to get to. It's seems like a good first issue and time sensitive for unblocking?

Also happy to keep making progress on task 2, which file would be effected?

@NoureldinYosri
Copy link
Collaborator Author

@shef4 the first task is the more tracktable of them. feel free tow work on it. the method should live in cirq-core/cirq/testing under consistent_unitary.py. you should create consistent_unitary.py since it doesn't exists and consistent_unitary_test.py for its tests.

the other tasks are not suitable as first issues. even the first is a bit too much so feel free to drop it if you feel that, but let me know if you do drop it.

@tanujkhattar tanujkhattar added good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add labels Jul 5, 2023
@NoureldinYosri
Copy link
Collaborator Author

NoureldinYosri commented Jul 5, 2023

Cirq-sync: accepted .. AI for myself I will refactor the description so that this issue would refer to only the first task and create new issues for the open ended problems

@shef4
Copy link
Contributor

shef4 commented Jul 10, 2023

Hey @NoureldinYosri I was wondering if I could get some feedback on my conssitent_unitary function.

from cirq.linalg.predicates import is_unitary
from typing import Any

def assert_unitary_is_consistent(val: Any):
    """Asserts that a value is unitary, clean qubits are set to |0> state, and borrowable qubits are restored to their original state."""
    
    # assert that value is unitary 
    assert is_unitary(val)

    # assert clean qubits are set to |0> state
    clean_state = cirq.PauliString(qubit=cirq.LineQubit(0))
    #TODO: collect list of clean qubits
    clean_qubits = [None]
    for qubit in clean_qubits:
        assert qubit == clean_state

    # assert borrowable qubits are restored to original state
    #TODO: collect list of orignial state of borrowable qubits
    original_states = [None]
    #TODO: collect list of borrowable qubits
    borrowable_qubits = [None]
    for i, qubit in enumerate(borrowable_qubits):
        assert qubit == original_states[i]

These are the assumptions I'm trying to look into to complete the function:

  1. The expected inputs for assert_unitary_is_consistent(val: Any) is only a channel of operations and gates.
  2. The original qubit state is stored in channel of operations and gate classes.
  3. comparing qubit states can be done using assert current_qubit == expected_qubit
    Are these assumptions correct?

I'm gonna look more into channel of operations and gates to see if I can find information on 2

@NoureldinYosri NoureldinYosri changed the title Open problems related to cirq.unitary and cirq.apply_unitaries protocols in the presence of ancillas Implement consistancy checks for unitary protocol in the presence of ancillas Jul 13, 2023
@NoureldinYosri
Copy link
Collaborator Author

@shef4 I started explaining how it should work but then found that it would be easier if I implement it. If you check #6196 you will find that the implementation mimics how #6112 updated cirq.unitary

@NoureldinYosri NoureldinYosri self-assigned this Jul 13, 2023
@shef4
Copy link
Contributor

shef4 commented Jul 13, 2023

@NoureldinYosri Okay, will do. Thanks for the guidance through the feature request, I learned a lot about the process though wish I could have been of more help haha. Will look for a "Good First Issue" to tryout

@NoureldinYosri
Copy link
Collaborator Author

completed in #6196

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. kind/feature-request Describes new functionality triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add
Projects
None yet
Development

No branches or pull requests

3 participants