-
Notifications
You must be signed in to change notification settings - Fork 2
feat: implement base structure for structured benchmark programs #35
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
base: main
Are you sure you want to change the base?
feat: implement base structure for structured benchmark programs #35
Conversation
burgholzer
left a comment
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.
Thanks @DRovara! Great work, gain 🙏🏼
I think I now gave this a pretty thorough review.
I tried to include explicit suggestions wherever I could so that you can directly batch them up on GitHub and commit them right away, which should also directly resolve the respective comments.
Some of the other comments might take a little bit of work, but it should not be too bad, hopefully.
| OPENQASM 3.0; | ||
| include "qelib1.inc"; | ||
|
|
||
| input int n; |
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.
Should we note somewhere that n must be greater than 1?
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.
Good point. Alternatively, we could also make it so that the register q has size n and the program just uses n+1 qubits total to also include the ancilla qubit.
I've just noticed the same logic applies to a lot of other benchmark programs, too.
In that case, maybe we really should just indicate that it requires at least 2 qubits here.
What would you say is the best way to do that? Just a comment?
| // Extract each bit: if 0, we need to flip the corresponding qubit | ||
| // Bit i is: (state >> i) & 1 | ||
| for int bit_pos in [0:num_controls-1] { | ||
| int bit_value = (state >> bit_pos) & 1; | ||
| if (bit_value == 0) { | ||
| x controls[bit_pos]; // Flip this control | ||
| } | ||
| } | ||
|
|
||
| // Apply fully-controlled gate (all controls must be |1⟩) | ||
| ctrl(num_controls) @ ry(angles[state]) controls[0:num_controls-1], target; | ||
|
|
||
| // Flip controls back | ||
| for int bit_pos in [0:num_controls-1] { | ||
| int bit_value = (state >> bit_pos) & 1; | ||
| if (bit_value == 0) { | ||
| x controls[bit_pos]; // Flip back | ||
| } | ||
| } |
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.
Just thinking out loud here: could this be somewhat handled with the negctrl modifier of OpenQASM?
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 don't think so - that was originally my idea when adding the code. However, I think the mix and match between ctrl and negctrl would cause issues. Imagine for the case where we search for the following structure:
Positive - Negative - Positive
among 3 control qubits.
The code would then have to be ctrl(1) negctrl(1) ctrl(1). Even if that is allowed (which I am not sure it is), that single instruction must work with the entire while loop for each iteration. So if the next structure we look for is
Positive - Positive - Negative
then how would the modifiers work? ctrl(2) negctrl(1) ctrl(?) - after all, the existence of the modifiers (even if not the parameters within the brackets) are compile-time constants. In that case, the best one could do is to have
ctrl(q1_pos) negctrl(!q1_pos) ctrl(q2_pos) negctrl(!q2_pos) ... ctrl(qn_pos) negctrl(!qn_pos) and then define each of these variables using bit-masks on the current loop index.
But even if we ignore the fact that that would be quite ugly (and likely not even allowed syntax), that would only work with a fixed number of qubits.
All that is to say: I don't think it would work with the negctrl modifier.
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
Co-authored-by: Lukas Burgholzer <[email protected]>
…ra/jeff into feat/structured-benchmark-programs
Description
This PR adds the base structure of the structured banchmarks that were proposed by RFC-0032.
In particular, this PR contains:
benchmarks/structuredserving as the base directory for benchmark programs andbenchmarks/_recipes/structuredproviding a directory to leave "recipes" used to generate benchmark programs for the sake of reproducibility.READMEfile in the base directory of the structured benchmark programs, which provides a list of currently implemented and missing benchmark programs.Mering this PR advances the current progress in the implementation of RFC-0032.