-
Notifications
You must be signed in to change notification settings - Fork 53
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
Future and upgrades to T-Complexity protocol #735
Comments
re: Adjoint: I consider this a pretty fundamental limitation of the existing t-counting protocol which subverts the decomposition hierarchy. Costs should depend on the costs of the callees. |
Well, the existing t-counting protocol says (a) "You either provide me the cost yourself" or (b) "I'll decompose you and try to figure it out myself". In the cirq land, the The issue arises in the scenarios where we want all of the following
Point (3) is new and needs more design. This has nothing to do with t-complexity protocol subverting the decomposition hierarchy. It's an optional optimization in step (2) which can be avoided by deleting the implementation of |
It's to support the case where we have the call graph callees but not a full decomposition |
That case is now already supported after #740 and again is unrelated to point (3) above. |
It would be impossible to override |
The design that needs attention here is whether we want protocols like We earlier used to do (a) and now we do (b) for It seems like in your original approach you wanted to do |
No, I don't think the bloq author has the authority to annotate a bloq with T counts unless they are direct callees. It's a leaky abstraction when you jump levels. I guess this is most similar to (b); but it begs the question why even have this method when we can never use it |
I agree. We should not have (c)
"Never use it" is a misclassification I think. We (can) use the hardcoded values for all forward cases and non-trivial adjoints / controls; which forms a large part of the codebase.
There are two goals I think
|
You'd have to guarantee that nothing in the bloq's call graph has or will ever have a custom adjoint or controlled implementation (that changes the t count) |
Right now, the
TComplexity
protocol is broken as described in #732This issue is to discuss the future of TComplexity protocol since there have been informal discussions and upgrades to either deprecate it in favour of the new generalized costs framework or make improvements to it and make it better supported.
First, I'd like to highlight that the
TComplexity
protocol; or an equivalentLogicalCounts
protocol that captures the most commonly used logical counts people care about when thinking about fault tolerant resource estimation, is extremely important and we should have an easy way (read single function call) to obtain this value for any givenbloq
. TheTComplexity
/LogicalCounts
class is also the input format to invoke the microsoft azure resource estimator.The current way to obtain this value without going via the t-complexity protocol is
a) Compute
sigma
viabloq.call_graph()
b) Call
t_counts_from_sigma(sigma)
assumingsigma
was computed without any fancy parameters (eg: no truncation of the call graph usingmax_depth
parameter; etc.)We need to have a single function call (eg:
bloq.t_complexity()
orbloq.logical_counts()
) that gives the desired counts. Inspecting the call graph should be done only when then bloq users need to get into then weeds of how exactly the logical counts were computed. Having agreed on this premise, let's lay down some requirements (I'll refer to the protocol as TComplexity for now, but we should probably change it tological_counts
in the future) -a) The most important requirement is that the it needs to be
_fast_
for the general case. Expecting users to overridebuild_call_graph
should be the recommended way but if users do not provide this decomposition, we should still be able to support fast computations of logical counts. The cirq stylet_complexity
uses a global cache to store and retrieve the logical counts of bloqs. This has an advantage of reusing any structure that the bloq decomposition has and ends up being sufficient for a majority of cases. This is because a lot of the times, the very high gate counts are present because we do phase estimation on a unitaryU
which ends up repeatingU
a lot of times.b) It should report more than just T-counts and should be sufficient as an "input" to our qec overhead methods defined in
bloqs/surface_code
. Some other useful logical counts are - (a) Toffoli counts (b) qubit usage (c) Counts of (rotation, eps) pairs instead of just number of rotations (d) Rotation depth (azure model uses this)Design options going forward:
(a) Delegate computation of
LogicalCounts
to the bloq call graph -> bloq counts construction. Biggest bottleneck here is performance I believe. For example: I implemented a naive version of this approach in #732, specifically_t_counts_for_bloq
called from_populate_flame_graph_data
and its VERY slow. Thephase_estimation_of_quantum_walk
notebook is a good example of bloqs for which I want to compute the T-Complexity and plot framegraphs. I can do the former easily, but the latter can be done only for very small cases. The biggest reason for the slowdown is that I need thesigma
for each node of the call graph and not just the root node. So every time I visit a node in the call graph, I do anO(n)
computation to compute thesigma
.(b) We improve the cirq style
t_complexity
method to not compute an explicit call graph and directly, implicitly, compute only the logical counts we care about. We've already demonstrated that it's pretty fast. There are some design challenges here; for exampleAdjoint(subbloq)
expectssubbloq
to accept anadjoint
parameter whensubbloq
does not have a customadjoint
implemented but not when thesubbloq
provides a custom implementation ofadjoint
. This seems like a design smell and is confusing at best (Eg: if a Bloq A decomposes into a bloq B whereB
has custom adjoint butA
does not thenA._t_complexity_
should accept anadjoint=True
but not pass on the flag toB._t_complexity_
. A and B areMultiAnd
andAnd
gates right now)cc @mpharrigan
The text was updated successfully, but these errors were encountered: