-
Notifications
You must be signed in to change notification settings - Fork 44
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
Single-qubit and Controlled Z rotations #1455
base: main
Are you sure you want to change the base?
Conversation
Yep you can use CZPow = And^dagger . ZPow . And |
# special angle ZPow gets turned into clifford or T | ||
rots = ((n - 3) * (n - 2)) // 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why this is failing specifically for n=123
(and by a lot). There's (n-1) phase gradients, each of size i
and we want to skip the first two (S, T). cc @tanujkhattar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The formula looks correct to me. Did you try debugging by checking which of the phase gradient bloqs results in lower rotation counts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't look at the individual ones. Do you mind taking a look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It fails from n=41
onwards. Any rotation with angle 0.5 / 2**b
with b >= 40
is getting ignored as the _ANGLE_ATOL = 1e-12
which is roughly 2**40
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oooh interesting
@tanujkhattar do you have any thoughts on the cirq simulation failure |
qualtran/_infra/controlled.py
Outdated
|
||
@staticmethod | ||
def get_single_reg_ctrl_system( | ||
ctrl_bloq: 'Bloq', ctrl_reg_name: str | ||
) -> Tuple['Bloq', 'AddControlledT']: | ||
"""A static method for helping explicitly write your own `get_ctrl_system`. | ||
|
||
Bloq authors can set up a controlled version of the bloq if the controlled bloq | ||
takes one additional control register with a known name. You can use this function to | ||
easily return the callable required by `get_ctrl_system`. | ||
|
||
Args: | ||
ctrl_bloq: The controlled version of the bloq | ||
ctrl_reg_name: The name of the new register that takes a control soquet. | ||
|
||
Returns: | ||
ctrl_bloq: The control bloq, per the `Bloq.get_ctrl_system` interface. | ||
add_controlled: A function that adds the controlled version of the bloq to | ||
a composite bloq that is being built, per the `Bloq.get_ctrl_system` interface. | ||
""" | ||
|
||
def adder( | ||
bb: 'BloqBuilder', ctrl_soqs: Sequence['SoquetT'], in_soqs: dict[str, 'SoquetT'] | ||
) -> tuple[Iterable['SoquetT'], Iterable['SoquetT']]: | ||
(ctrl_soq,) = ctrl_soqs | ||
soqs = {ctrl_reg_name: ctrl_soq} | in_soqs | ||
soqs = bb.add_d(ctrl_bloq, **soqs) | ||
ctrl_soqs = [soqs.pop(ctrl_reg_name)] | ||
return ctrl_soqs, soqs.values() | ||
|
||
return ctrl_bloq, adder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be replaced with the new get_ctrl_system_1bit_cv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep this is some vestigial code where I wanted to try my hand at factoring it out :) thanks for actually doing it
|
||
|
||
@frozen | ||
class CRz(Bloq): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please override the ctrl system for this (and other controlled bloqs), and add a test to verify gate counts for CRz().controlled()
(should be CRz() + And() + And.adjoint()
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why would I need to override the ctrl system? there isn't a native CCRz bloq
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because right now, CRz.controlled
would return Controlled(CRz)
, i.e. does not merge the two controls using And
.
The second case of get_ctrl_system_1bit_cv
essentially handles this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(sync discussion) I did not notice the decomposition, in which case the override is not needed.
But I still think it's good to override to get a much more cost effective circuit. The current CRz(2*pi*t).controlled()
gives
toffoli: 2, and_bloq: 2, clifford: 2, rotation: 2, measurement: 2
But with the override it becomes
and_bloq: 1, clifford: 3, rotation: 2, measurement: 1
the subbloq CRz(2*pi*theta)
for reference:
full code
import attrs
import numpy as np
import sympy
from qualtran import Bloq, DidNotFlattenAnythingError
from qualtran.bloqs.basic_gates import CRz
from qualtran.bloqs.mcmt import And
from qualtran.drawing import show_bloq
from qualtran.resource_counting import get_cost_value, QECGatesCost
def display_bloq_and_cost(bloq, *, flat=True):
def _pred(binst):
return not binst.bloq_is(And)
cbloq = bloq.as_composite_bloq()
if flat: cbloq = cbloq.flatten(pred=_pred)
show_bloq(cbloq, 'latex')
cost = get_cost_value(bloq, QECGatesCost())
print(cost)
theta = 2 * np.pi * sympy.Symbol("t")
crz = CRz(theta)
display_bloq_and_cost(crz.controlled())
@attrs.frozen
class CRzWithOverride(CRz):
def get_ctrl_system(self, ctrl_spec):
from qualtran.bloqs.mcmt.specialized_ctrl import get_ctrl_system_1bit_cv_from_bloqs
return get_ctrl_system_1bit_cv_from_bloqs(
self,
ctrl_spec,
current_ctrl_bit=1,
bloq_with_ctrl=self,
ctrl_reg_name='ctrl'
)
display_bloq_and_cost(CRzWithOverride(theta).controlled())
print(cost.total_t_count())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that can be done in a follow-up. I don't think it's worth blocking this PR any more than it already has been
phase_op = phase_op.controlled(ctrl_spec=CtrlSpec(cvs=self.control_val)) | ||
phase_op = phase_op.controlled() | ||
if self.control_val == 0: | ||
costs[XGate()] = 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be += 2
? (as there's already an XGate()
in L186)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's guarded by if control_val is None
and this line test if control_val == 0
Update single-qubit Z rotations
ZPowGate
andRz
. These are atomic operations.Introduce actual constructions for
CZPowGate
andCRz
for better resource counting. The previous hack would just count them as "one rotation". Now the former is a rotation and two Toffolis (TODO: can these beAnd
?); the latter is two rotations and two cliffords.Connect the single-qubit gates to their controlled versions via the controlled protocol.
There's still hacks for Controlled-S and Controlled-T, which I will port in a follow-up.
How should we handle X and Y rotations? One possibility is to make them compile to ZPow/Rz. Otherwise: duplicate the controlled construction for each flavor.
@tanujkhattar there are two tests failing that I'd like your help with
controlled_by
which is causing breaking simulation tests in a weird way. This triggers BloqAsCirqGate.__pow__ probably broken #1446 but even after hack-fixing that it runs into a different issue. Switching to qualtran tensor contraction fixes the test but there may be outstanding issuestest_get_flame_graph_data_qft_textbook
has different numbers now but I don't know how to verify that these are correct.