Skip to content

Don't silently omit counts from missing callees #1311

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

Closed
wants to merge 3 commits into from

Conversation

mpharrigan
Copy link
Collaborator

Addresses the buggy part of #1290 by re-raising the error. Better error messages from Controlled(Atomic).

@mpharrigan mpharrigan requested a review from anurudhp August 20, 2024 16:26
@mpharrigan mpharrigan changed the title Don't silently omit counts from missing calees Don't silently omit counts from missing callees Aug 20, 2024
Copy link
Contributor

@anurudhp anurudhp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we distinguish between atomic bloqs and bloqs where the decomposition failed? Because in the former we treat it as a leaf bloq, and in the latter we cannot compute the correct cost. Is my understanding correct?

try:
sub_cg = self.subbloq.build_call_graph(ssa=ssa)
except DecomposeTypeError as e1:
raise DecomposeTypeError(f"Could not build call graph for {self}: {e1}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raise from?

@mpharrigan
Copy link
Collaborator Author

Do we distinguish between atomic bloqs and bloqs where the decomposition failed? Because in the former we treat it as a leaf bloq, and in the latter we cannot compute the correct cost. Is my understanding correct?

If it's a leaf bloq, QECGatesCost should have already determined its cost before trying to peer into its decomposition. If you get to the point in the code where you're trying to look at callees to determine costs: if it's a DecomposeTypeError then it's an error since we've reached an atomic, leaf bloq that we don't know how to count; if it's a DecomposeNotImplementedError then there are costs that should be from the decomposition but there is no decomposition. So for resource counting, those are both an error condition where we can't return resource counts

@anurudhp
Copy link
Contributor

Ah, makes sense, thanks!

@mpharrigan
Copy link
Collaborator Author

I was going to factor out this change from the fixes i've been making to QECGatesCost but they're somewhat intertwined since this change reveals the failures that need some of the other fixes. Moving to draft

@mpharrigan
Copy link
Collaborator Author

subsumed by #1313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants