Divergent behaviour of controlled: GateWithRegisters.controlled vs. Bloq.controlled

[anurudhp]

Ran into this issue while testing GQSP's call graph. Minimal code to reproduce:

import numpy as np
from qualtran.bloqs.generalized_qsp_test import RandomGate

U = RandomGate.create(bitsize=1, random_state=np.random.RandomState(42))
print(U.call_graph()) # works
print(U.controlled().call_graph()) # fails

output:

(<networkx.classes.digraph.DiGraph object at 0x7fc8491550d0>, {RandomGate(bitsize=1, matrix=array([[ 0.27702459-0.13059069j, -0.76350488+0.56856287j],
       [ 0.36122517+0.88074958j,  0.1700831 +0.25469254j]])): 1})

Traceback (most recent call last):
  File "test.py", line 6, in <module>
    print(U.controlled().call_graph())
          ^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'ControlledGate' object has no attribute 'call_graph'

[mpharrigan]

I'll look at this. Since cirq.Gate and Bloqboth have a method named controlled we have to pick the most useful default.

A workaround if you have access to the source code for the problematic gate is to call the correct one

class MyGWR(GateWithRegisters):
  ...

  def controlled(self, *args, **kwargs):
    return Bloq.controlled(*args, **kwargs)

[anurudhp]

Thanks!

So I was using this for Hamiltonian simulation by GQSP, which uses QubitizationWalkOperator. @tanujkhattar, I believe it would be a major refactor to make it and all its dependencies Bloqs?

[fpapa250]

Running into this error in #780. Specifically with MontgomeryModInv() bloq whose call graph involves Add().controlled() and LinearDepthGreaterThan().controlled().

Code to reproduce:

from qualtran.drawing import show_call_graph
from qualtran.bloqs.factoring.mod_mul import MontgomeryModInv

bloq = MontgomeryModInv(bitsize=5, p=6)

graph, sigma = bloq.call_graph()
show_call_graph(graph)

[mpharrigan]

reflecting on this, we should probably change the default GateWithRegisters.controlled to be the bloqs one and then use the workaround for cases when we need the cirq behavior

[tanujkhattar]

We should ideally never need the cirq behavior now that we have a mapping from the Controlled bloq to cirq.ControlledGate and Controlled bloq supports simulations.

For convenience with cirq and bloq APIs and to avoid potential performance regressions; I'd suggest we make Controlled bloq a GateWithRegisters (eg: this would be useful to directly use the unitary of the subbloq when subbloq is GateWithRegisters)