-
Notifications
You must be signed in to change notification settings - Fork 920
Adding CControlled function to the canon, w/ unit tests. #53
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.
Looks good!
All in all nicely done. Just a couple of comments about the documentation, and making sure that all introduced code is covered by a test.
/// ## bit | ||
/// a boolean that controls whether op is applied or not. | ||
/// ## target | ||
/// Qubit to which the operation is applied. |
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'd suggest to edit that, since the target does not necessarily have to be a qubit.
Same for all documentations below.
/// | ||
/// # Type Parameters | ||
/// ## 'T | ||
/// The type of the operation to be conditionally applied. |
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.
Being a bit pedantic here, I'd modify the description to say the input type of the operation, since the type of the operation indeed includes the entire signature.
Same for all documentations below.
/// | ||
/// # Type Parameters | ||
/// ## 'T | ||
/// The type of the operation to be conditionally applied. |
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.
Same here: 'T is the input or argument type of the operation.
controlled adjoint auto | ||
} | ||
|
||
operation CControlledActual(op : (Qubit => ()), target : Qubit[]) : () { |
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.
Do you mind adding the same thing for CControlledA, CControlledC, CControlledCA (I think putting them in the same test is fine)? Otherwise I can quickly add them - just for the sake of always having at least one test that covers every piece of code. I apologize for the hassle of having to make the distinction for operations supporting various functors in the first place; we are working on making this more convenient.
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.
Fantastic! Thanks!
* Adding CControlled function to the cannon, w/ unit tests. * Fixed doc strings and tests as suggested in PR
* Fixing doc gen call (microsoft#39) * fixing doc gen call * adding a test for docs generation * need to generate docs for source files only such that we can build the docs for muliple dlls without interferences * forgot to check if the dictionary exists before clearning it in test * Updating qdk to latest version
) * Updating QDK to latest version. (microsoft#53) * Fixing doc gen call (microsoft#39) * fixing doc gen call * adding a test for docs generation * need to generate docs for source files only such that we can build the docs for muliple dlls without interferences * forgot to check if the dictionary exists before clearning it in test * Updating qdk to latest version * Fixing vs code extension build script on linux (microsoft#56) * fixing a stackoverflow for large files (microsoft#55) * Build VS Code dependencies from either pwsh or powershell. (microsoft#59) * Add automatic indentation to Visual Studio extension (microsoft#60) * Add automatic indentation to Visual Studio extension * Find the indentation of the last non-empty line The previous line can be empty, which would reset the indentation to 0 if e.g. you pressed enter twice in a row. Loop backwards to find the last non-empty line and use that line's indentation instead. * Remove assumption from GetLastNonEmptyLine * Move QsSmartIndentProvider to the top of the file * Flip order of ternary operator in GetIndentation * Add doc comment to GetDesiredIndentation * Add note about nullable return type of GetDesiredIndentation * Show signature help for the first argument (microsoft#63) We also trigger signature help on "(" * Always log exceptions from file queries in debug mode (microsoft#70) * Handling specialization redeclarations (microsoft#85) * Add install template and new project commands to VS Code extension. (microsoft#79) * Compile time errors for overflowing int and double literals (microsoft#87)
Added new function and operation with unit tests.