-
Notifications
You must be signed in to change notification settings - Fork 72
[QECGatesCost] strict & account for more leaf bloqs #1313
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
Conversation
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.
lgtm % concerns using bloq.eps
I don't see a comment, did you try to leave one? From offline: I'll use machine precision to compare the angles |
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.
oops, added the comment now.
if isinstance(b, (Rz, Rx, Ry)): | ||
if is_symbolic(b.angle): | ||
return False # Symbolic rotation | ||
if np.any(np.abs(b.angle - _T_ANGLES) < _ANGLE_ATOL): | ||
return True # T hidden in a rotation bloq | ||
return False |
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 think bloq_is_t_gate
should return True only if the unitary of the bloq is equal to that of a T
gate. For other single qubit rotations, we should have default decompositions; which a user can choose to override if needed; to avoid undercounting cliffords.
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.
we generally don't have "default" decompositions that one can override.
I can change this function name to bloq_is_t_like
. Counting Rx(pi/4) as 1 T gate is much more accurate than 1 "rotation", so I'm going to proceed with the change. We can add a decomposition of Rx to Rz later or change the workflow where a re-write rule is responsible for simplifying any Rx(theta) to X/4 or whatever we like (and remove this itchy logic from the gate counting code).
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.
it's also unclear (to me) how we'd want to count cliffords for a situation like Rx(pi/4). Using an auto-correcting circuit like Fig 17 of game-of-surface-codes implies you can do any pauli with the same number of gates
The first commit is #1311 ; which can't be merged independently because it reveals errors that this PR fixes.