Closed tanujkhattar closed 2 months ago
+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
.
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())
Ah yeah, if we add a separate GWROperation
, it solves the problem I described (can just use controlled
)
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?
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.
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 forBloq.get_ctrl_system
s.t. in most cases (where the assumption that all ctrl registers occur before all original registers) users can simply overrideBloq.controlled()
method instead of overridingBloq.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 https://github.com/quantumlib/Qualtran/pull/927