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.
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
Permutation Bloq #1110
Permutation Bloq #1110
Changes from 1 commit
d32e1b1
3e1b4b5
bb3af07
8dd9f27
73900ef
7f5e79c
228eb84
aa3a169
3f31d1e
b2bd6ba
17868c5
f44ecf6
5df60b3
64b109a
bea33f2
e2a2385
cf091d0
62508e6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
If
k
is aSymbolicInt
should we restrictdtype
to beQUInt
/QInt
? Looking at the implementation below, it seems like this would also work for other types whenk
is not necessarily an integer.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 shouldn't do
n
MCX
gates to don
bit flips when number of controls > 1Instead, divide it into 3 cases:
MultiAnd(cvs=self.cvs)
with the temporary ancilla as the target. Then reduce to Case(2) with the temporary ancilla as control.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.
In fact, I'd suggest we only support Case (1) and (2) and reducing Case (3) to Case (1) is a general strategy used for all multi controlled bloq so we can add a separate Bloq that does this reduction (maybe at a later point)
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 simplified this to only support a single
control_val
, as #1131 will handle the CtrlSpec -> QBit part.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.
can you add a note that this is just a negative-control CNOT so that in the future, if and when we add a
cv
attribute toCNOT
we can replace itThere 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.
nit: I generally prefer inverting this so the method isn't as indented
That being said --- my original design for the call graph protocol was always to provide the counts you'd expect to find in e.g. a paper, which would use the
is_symbolic
path even for concrete numbers. Then you can reconcile them in e.g. the unit test by using thegeneralizer
argument and built-ingeneralizer
functions. Maybe I'll open an issue because I know this sort of thing has come up in other placesThere 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.
Inverted the method.