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

Add a bb.add_flat(bloq, flat_list_of_soqs) and use it to provide a sensible default for bloq.get_ctrl_system infrastructure #931

Open
tanujkhattar opened this issue May 8, 2024 · 5 comments

Comments

@tanujkhattar
Copy link
Collaborator

Having a method on BloqBuilder that can accept a flat list of soquets and do the necessary reshapes would allow us to provide a default implementation for Bloq.get_ctrl_system s.t. in most cases (where the assumption that all ctrl registers occur before all original registers) users can simply override Bloq.controlled() method instead of overriding Bloq.get_ctrl_system. The former is much more nature for users.

This would also allow us to avoid the complexity of having specialized base classes for defining singly controlled bloqs like the one added in #927

@anurudhp
Copy link
Contributor

anurudhp commented May 8, 2024

+1

On top of this, I think we should add Bloq.build_controlled_bloq which is called by get_ctrl_system which in turn is called by controlled (similar to build_composite_bloq and decompose_bloq). Because the GateWithRegisters.controlled method exposes a much more complex API than needed, users should only see the final CtrlSpec.

@tanujkhattar
Copy link
Collaborator Author

I'm thinking of being in a world where users can simply override .controlled instead of overriding .get_ctrl_system for most of the standard cases. I'm not sure what's the value of Bloq.build_controlled_bloq over simply Bloq.controlled ?

Because the GateWithRegisters.controlled method exposes a much more complex API than needed, users should only see the final CtrlSpec.

I think that's a temporary interface which should be deprecated. GateWithRegisters should always only expose the CtrlSpec API and we should add a GWROperation that derives from GateOperation and exposes the cirq-style .controlled_by API, converts it to a CtrlSpec using functionality present in GateWithRegisters._get_ctrl_spec() (we'll move the function to the new operation) and then delegates to GateWithRegisters.controlled(ctrl_spec())

@anurudhp
Copy link
Contributor

anurudhp commented May 8, 2024

Ah yeah, if we add a separate GWROperation, it solves the problem I described (can just use controlled)

@ncrubin
Copy link
Collaborator

ncrubin commented May 9, 2024

Is this really necessary? It feels like yet another layer between the user and the bloq/circuit. IMHO cirq has too many protocols between a list of gates and an simulator and this feels similar. Maybe you can expand on why this default is necessary to begin with?

@mpharrigan
Copy link
Collaborator

I'm wary of adding flat and position-dependent args to bloqs. One of the selling points is that using (the equivalent of) keyword args everywhere removes an entire class of errors. This comes at the expense of slightly more complicated logic "on the back-end" for places where wiring things up needs to be automated.

Defining controlled versions of bloqs is a place where this distinction gets a bit muddled. I did think about it for some time, and the get_ctrl_system thing is what I came up with if we want to support arbitarary CtrlSpec. Maybe we can have a carve-out (special protocol) for one qubit control that can be simpler.

But I think this all needs to be carefully thought out. I'd like to see the custom controlled versions of gates implemented using the current system so that any refactoring can be accompanied by clear examples of how/whether it actually cleans up the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants