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

Allow protocols to work on raw arrays #7050

Open
daxfohl opened this issue Feb 8, 2025 · 4 comments
Open

Allow protocols to work on raw arrays #7050

daxfohl opened this issue Feb 8, 2025 · 4 comments
Labels
good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. good for learning For beginners in QC, this will help picking up some knowledge. Bit harder than "good first issues" kind/feature-request Describes new functionality triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@daxfohl
Copy link
Collaborator

daxfohl commented Feb 8, 2025

Is your feature request related to a use case or problem? Please describe.

When working on #7048, was surprised that unitary(val) and apply_unitary(val, args) didn't accept raw numpy unitaries for val. This made it necessary to special case a couple isinstance(np.ndarray) checks in the consumers of those functions. There is also an already-existing redundant function in apply_mixture that can be removed if apply_unitary can accept raw numpy unitaries. If accepted, this could be a good first issue for someone.

Describe the solution you'd like

I started this for apply_unitary in #7039, but closed the PR because a) I think it's a maintainer decision as to whether the feature is desired, how large the scope should be, and whether it can be delivered incrementally or should go all-at-once, and b) I think it would be a good first issue for someone to learn about quantum operators in general, and cirq's implementation of protocols in particular.

As mentioned in the closing comment #7039 (comment), the scope is open-ended.

Closing for now. Seems like if we do this, then we should also do unitary and has_unitary. Which, then may as well do mixture and kraus series, maybe act_on, and whatever else comes up too. Support raw Python arrays? Sympy?....

[optional] Describe alternatives/workarounds you've considered

Duplicate code, instanceof checks.

[optional] Additional context (e.g. screenshots)

As part of this, I noticed the protocols have different techniques. Some loop through an array of strategies, others are more imperative. Some use dynamic invocations of the fallback strategies, whereas others call the fallback protocols directly. It'd be good to improve consistency in these when working through them.

What is the urgency from your perspective for this issue? Is it blocking important work?

P3 - I'm not really blocked by it, it is a suggestion based on principle

@daxfohl daxfohl added good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. good for learning For beginners in QC, this will help picking up some knowledge. Bit harder than "good first issues" kind/feature-request Describes new functionality triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Feb 8, 2025
@NikolaiLong
Copy link

Hello @daxfohl, I'm new to this project and have just set up my environment locally with all tests passing. I have many years of experience with python and I would like to work on this issue. Could you assign it to me please? Thank you!

@daxfohl
Copy link
Collaborator Author

daxfohl commented Feb 21, 2025

I'm not a maintainer, and the issue is still in triage state. Feel free to do it, but there is no guarantee that the issue will be accepted. Maybe attend the Wednesday sync if you'd like to petition for it.

@NikolaiLong
Copy link

Thank you for the clarification. I'll attend Wednesday's sync.

@NoureldinYosri NoureldinYosri removed the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Mar 5, 2025
@mhucka mhucka added the triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on label Mar 5, 2025
@mhucka
Copy link
Contributor

mhucka commented Mar 5, 2025

Discussed during Cirq Cynq 2025-03-05: accept as a worthwhile change. Limit the data types to things that can be turned into NumPy arrays. (Users will have to convert other things like SymPy objects to NumPy arrays if they need to do that.)

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. good for learning For beginners in QC, this will help picking up some knowledge. Bit harder than "good first issues" kind/feature-request Describes new functionality triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Development

No branches or pull requests

4 participants