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

Propagate rotation epsilon in gate cost #1250

Open
mpharrigan opened this issue Aug 5, 2024 · 3 comments · May be fixed by #1255
Open

Propagate rotation epsilon in gate cost #1250

mpharrigan opened this issue Aug 5, 2024 · 3 comments · May be fixed by #1255

Comments

@mpharrigan
Copy link
Collaborator

The t_complexity protocol ignores the eps parameter when counting rotations.

The solution is to enhance the QECGatesCost cost and associated dataclass GateCounts to count rotation/eps pairs.

@anurudhp
Copy link
Contributor

anurudhp commented Aug 5, 2024

Sample code snippet:

import sympy
from qualtran.bloqs.basic_gates import ZPowGate

t, eps = sympy.symbols(r"t \epsilon")
bloq = ZPowGate(exponent=t, eps=eps)
print(bloq.t_complexity())

Prints TComplexity(rotations=1).

One could do bloq.t_complexity().t_incl_rotations(eps), but this eps is not the same as the bloq's eps (which in general could be different for each subbloq).

@anurudhp anurudhp linked a pull request Aug 7, 2024 that will close this issue
@mpharrigan mpharrigan added this to the v1.0 milestone Aug 8, 2024
@tanujkhattar
Copy link
Collaborator

Some history:

eps for rotations was introduced in the early days of Qualtran and I think is a useful parameter when constructing rotation bloqs. I agree it's probably not propagated correctly everywhere (though I remember implementing and correctly propagating eps in qvr, qft and hamming weight phasing bloq) -- we should file issues and try to get that fixed.

Originally, the rotation bloqs had a _t_complexity_ method implemented and would return the number of T gates scaling with log(1/eps); so the T-counts for call graphs with rotation bloqs would take the eps in account. (eg: #469)

As part of unifying / deprecating the T-complexity protocol; a temporary fix was to return rotations=1 for all rotation bloqs and add a method on the TComplexity class to convert rotations into T-gates. Eventually, the idea was to use the new costing framework to keep track of bloq counts and then use the eps to convert rotation bloqs to t-gates like we used to do originally. Here are some relevant discussions -
#662 (comment)
#678 (comment)

Using a single / hardcoded eps to convert all rotation bloqs to t gates was never meant to be a permanent solution. We've just not gotten around completing the migration yet I guess.

@tanujkhattar
Copy link
Collaborator

@mpharrigan As per our discussion offline, it'll be nice to have this as part of the v1.0 milestone.

@tanujkhattar tanujkhattar modified the milestones: After v1.0, v1.0 Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants