[Costs] Qubit Counts #969
[Costs] Qubit Counts #969
Conversation
|
|
||
| from ._costing import GeneralizerT, get_cost_value, get_cost_cache, query_costs, CostKey, CostValT | ||
|
|
||
| from ._qubit_counts import QubitCount |
There was a problem hiding this comment.
why is it a private module?
There was a problem hiding this comment.
This is to force imports of the form
from qualtran.resource_counting import QubitCount
instead of
from qualtran.resource_counting.qubit_counts import QubitCount
There was a problem hiding this comment.
the underscore isn't necessary though right? You could still do from .qubit_counts import QubitCount no? Or are you saying you don't want me to import from qubit_counts ever?
fdmalone
left a comment
fdmalone
left a comment
There was a problem hiding this comment.
LGTM. I'll need to play around with it some more to check the chemistry algorithms.
tanujkhattar
left a comment
tanujkhattar
left a comment
There was a problem hiding this comment.
Question about scalability, do we have any plans to address it? Is it worth discussing the design choices again and whether my_static_costs should be really only overridden for the leaf bloqs?
| try: | ||
| cbloq = bloq.decompose_bloq() | ||
| logger.info("Computing %s for %s from its decomposition", self, bloq) | ||
| return _cbloq_max_width(cbloq._binst_graph, get_callee_cost) | ||
| except (DecomposeNotImplementedError, DecomposeTypeError): | ||
| pass |
There was a problem hiding this comment.
This is a good example of a cost which doesn't scale for large bloqs, as I had raised a concern for in your previous PR (cc #957)
Do we have a plan to make this scalable? Do we plan to do isinstance based dispatch for special bloqs as you had proposed in #913 (comment) ?
There was a problem hiding this comment.
Here is another example for the hubbard model -
There was a problem hiding this comment.
for something like Add or QROM (which is the slow part in hubbard model) you can annotate the static qubit counts statically
There was a problem hiding this comment.
I agree. And I think the right way to annotate these static costs is to express the cost as a formula of the cost of its callees. For example - the SelectSwapQROM can express its qubit counts in terms of qubit_count(QROM) and qubit_count(SwapWithZero) bloqs. Similarly, phase estimation can express its cost in terms of qubit_count(U); instead of specifying a constant (which is not even possible for phase estimation since U is unknown and decomposing phase estimation can be costly in general, since you end up with a circuit where the depth is at least 2 ** size(phase_register))
For this to scale, it would be really nice if my_static_costs has access to the cache and can forward it to the get_cost_value method to get the cost of its callees. Since we are designing the API right now, it'll be easier to make this change and be future proof so I'd suggest you to reconsider this proposal.
tanujkhattar
dismissed
their stale review
After many offline discussions it seems its hard to reach a consensus. I'll dismiss my blocking review so we can make progress and potentially revisit later if needed.
3 participants
This introduces a
CostKeyto count qubits. Part of #899.