-
Notifications
You must be signed in to change notification settings - Fork 51
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
[Costs] Framework #913
Merged
mpharrigan
merged 15 commits into
quantumlib:main
from
mpharrigan:2024-04/generic-costs-framework
May 15, 2024
Merged
[Costs] Framework #913
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
da80676
costing framework
mpharrigan 79b85b1
docstrings and linting
mpharrigan a3a2f09
lint
mpharrigan e8de6e9
Merge branch 'main' into 2024-04/generic-costs-framework
mpharrigan cce03f3
python 3.10 type issues
mpharrigan 1e63fa6
Merge remote-tracking branch 'mine/2024-04/generic-costs-framework' i…
mpharrigan a2bb2ed
Merge branch 'main' into 2024-04/generic-costs-framework
mpharrigan 5b9ad8b
lint
mpharrigan 8fa6e2a
Merge branch 'main' into 2024-04/generic-costs-framework
mpharrigan 80dfbac
Merge remote-tracking branch 'origin/main' into 2024-04/generic-costs…
mpharrigan 88ea840
part b
mpharrigan d3c5461
copyright
mpharrigan 2c81d44
mypy
mpharrigan 32a30a7
lint
mpharrigan d1196dd
Merge remote-tracking branch 'origin/main' into 2024-04/generic-costs…
mpharrigan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
# Copyright 2024 Google LLC | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# https://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
from typing import Any, Sequence, Set, Tuple | ||
|
||
from attrs import field, frozen | ||
|
||
from qualtran import Bloq, Signature | ||
from qualtran.resource_counting import BloqCountT, CostKey, SympySymbolAllocator | ||
|
||
|
||
def _convert_callees(callees: Sequence[BloqCountT]) -> Tuple[BloqCountT, ...]: | ||
# Convert to tuples in a type-checked way. | ||
return tuple(callees) | ||
|
||
|
||
@frozen | ||
class CostingBloq(Bloq): | ||
"""A bloq that lets you set the costs via attributes.""" | ||
|
||
name: str | ||
num_qubits: int | ||
callees: Sequence[BloqCountT] = field(converter=_convert_callees, factory=tuple) | ||
static_costs: Sequence[Tuple[CostKey, Any]] = field(converter=tuple, factory=tuple) | ||
|
||
@property | ||
def signature(self) -> 'Signature': | ||
return Signature.build(register=self.num_qubits) | ||
|
||
def build_call_graph(self, ssa: 'SympySymbolAllocator') -> Set['BloqCountT']: | ||
return set(self.callees) | ||
|
||
def my_static_costs(self, cost_key: 'CostKey'): | ||
return dict(self.static_costs).get(cost_key, NotImplemented) | ||
|
||
def pretty_name(self): | ||
return self.name | ||
|
||
def __str__(self): | ||
return self.name | ||
|
||
|
||
def make_example_costing_bloqs(): | ||
from qualtran.bloqs.basic_gates import Hadamard, TGate, Toffoli | ||
|
||
func1 = CostingBloq( | ||
'Func1', num_qubits=10, callees=[(TGate(), 10), (TGate().adjoint(), 10), (Hadamard(), 10)] | ||
) | ||
func2 = CostingBloq('Func2', num_qubits=3, callees=[(Toffoli(), 100)]) | ||
algo = CostingBloq('Algo', num_qubits=100, callees=[(func1, 1), (func2, 1)]) | ||
return algo |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
# Copyright 2024 Google LLC | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# https://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
from qualtran.bloqs.for_testing.costing import make_example_costing_bloqs | ||
from qualtran.resource_counting import format_call_graph_debug_text | ||
|
||
|
||
def test_costing_bloqs(): | ||
algo = make_example_costing_bloqs() | ||
g, _ = algo.call_graph() | ||
assert ( | ||
format_call_graph_debug_text(g) | ||
== """\ | ||
Algo -- 1 -> Func1 | ||
Algo -- 1 -> Func2 | ||
Func1 -- 10 -> Hadamard() | ||
Func1 -- 10 -> TGate() | ||
Func1 -- 10 -> TGate(is_adjoint=True) | ||
Func2 -- 100 -> Toffoli() | ||
Toffoli() -- 4 -> TGate()""" | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
One issue here is that
my_static_costs
method does not have access to the costs cache. So, if it were to do any non-trivial computation where it needs to compute its cost in terms of the cost of its callees, then it wouldn't be able to pass on the cache toget_cost_value
method.We had faced this issue with
TComplexity
protocol and that's why we ended up using a global cache. In this framework, we should maybe accept a cache dictionary as another argument to this method since the call site already has it ?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.
ah the intent here is for
my_static_costs
to be used rarely and only for truly static costs that you can just declare. Consider it the generic analog to the_t_complexity_
override where you're really only allowed to fill in numbers.The cache is available to the
compute
method, which is analogous to every strategy except thet_complexity._from_explicit_annotation
strategy in the t complexity protocol.I'll update some docstrings to clarify the expected use of
my_static_costs
.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 understand that, but I think there's value in supporting the use cases where
my_static_costs
needs to do something more complex than specifying just numbers.An example would be supporting the cost key for circuit
depth
. Thecompute
method would correspond to callingbloq.decompose_bloq()
and then (one strategy would be to) find the longest path in a weighted DAG where the weights are given by depth of each subbloq. This will take time at least O(N) if the decomposition hasN
nodes (unlike other costs like T-complexity, which scale as O(M) where M is number of unique callees and often M << N)Now, if we stick to the strategy where
my_static_costs
specifies just numbers andcompute
works as described above, the depth computation would be prohibitively slow for a wide variety of bloqs where decomposing the bloq would result in a large number of nodes. Some concrete examples are bloqs based on unary iteration, arithmetic circuits like adders etc.In all these examples, as bloq authors we very well know the circuit structure and can easily annotate
my_static_costs
. But it's not enough to specify "a number" - we need themy_static_costs
to be an expression in terms of costs of its callees. For unary iteration based bloqs, themy_static_costs
would be something likec * N + sum(get_bloq_cost(bloq_for_nth_operation(n)) for n in range(self.selection_range))
- i.e. a constant for the unary iteration tree + depth for each controlled operation. For arithmetic circuits, we'd likely want to specify costs as a function ofget_cost(Toffoli)
; which users can choose to specify by providing the cost dictionary since a Toffoli can have multiple different decompositions optimizing depth vs T-counts etc. and users can choose which decomposition should be used by providing a cost for depth ofToffoli
.Tl;Dr - I think its restrictive to assume that
my_static_costs
would only ever specify truly static costs and I think there's value in making it extensible to support the use case where it'll recursively need and use costs for it's callees.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 idea is to group any of this logic in the compute method, which can dispatch based on the type of bloq (analogous to how we provide costs for leaf bloqs in the existing t_complexity protocol).
We should also aim to reduce the number of bloqs where doing an O(N) operation is considered prohibitively costly. In your example, you still need to iterate over
self.selection_range
bloqs. I guess the presumption is that the constant term absorbs a number of bit-twiddling subbloqs that meaningfully exceeds the selection_range; but we should probably drive our efforts towards encoding the "simple" structure of unary iteration in the bloq decomposition hierarchyThere 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.
Yeah, I think that would just not be feasible for the cost types that need to look at the decompose composite bloq instead of just the bloq counts. I've raised this concern before and I provide a very concrete exampel above.
For this specific example, the presumption is that building the composite bloq can be much slower than iterating over the callees, even if both of these take
O(N)
time. #609 (comment) was an example of how much of a different this can make.Well, that's a very optimistic goal and I don't think it aligns well with the rest of the philosophy of Qualtran. One of the reasons we have the concept of a call graph is because we think there will be cases where constructing a composite bloq in
O(N)
time would be prohibitively more expensive than just specifying a list of callees and their bloq counts.The same logic applies to costs that depend upon the structure of the decomposed composite bloq, like
depth
. There will be cases where computing the composite bloq and then processing it to figure out the cost would be prohibitively more expensive than writing an expression for computing the cost in terms of the callees. For costs like T-complexity, this "expression" is simple and can be generalized and put as part of thecompute
method but for costs likedepth
this "expression" needs to live inside the bloq. That place can bemy_static_costs
(or we can rename it to be more descriptive for this use case). But as the design stands right now, we don't support this flow at all where users can write the expression for computing a cost for a bloq (in terms of its callees) as part of the bloq itself.For some more concrete numbers, here are the timings to construct a composite bloq for a
1000
and5000
bit adder. You can see this is already getting very expensive and these are numbers which we encounter often.So now, if a user were to compute the cost of an adder for
depth
, they would either not overridemy_static_costs
and we'll pay the time penalty of callingadder.decompose_bloq()
(which is huge) OR the user would need to encode an expression that hardcodes the depth of subbloqs likeAND
gates; which isn't ideal and is similar to what we were doing in T-complexity and got rid of by introducing the call graph hierarchy. The ideal solution here would be to allow users to specify things likeadder_depth = c1 * cost(And()) + c2 * cost(And().adjoint()) + c3 * cost(CNOT())
wherec1
,c2
andc3
are constants which are known to the bloq authors because they know the structure of the decomposition. A function similar tomy_static_costs
can be used for such annotations.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.
You can put that logic in the
compute
method.Remember: the extensibility axis is to support adding more costs. This is particularly acute for circuit depth, which isn't the most important cost for certain architectures.
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.
This means every time a bloq author writes a bloq which is slow to decompose for large instances, they would have to update the
compute
method of the cost key - this does not scale at all (unless we assume all bloqs are authored by us and we maintain a central repository of these if / else overloads in the compute method of the cost key).The argument you have given above completely ignores the ease of extensibility for more costs. Since the PR is already merged now, I'll open a new issue to track this so that whenever we end up implementing a cost like
depth
which depends upon the structure of decomposition, we can revisit this discussion.