-
Notifications
You must be signed in to change notification settings - Fork 936
Add a very basic implementation of Grover's algorithm #224
Conversation
msoeken
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.
Hi @thomashaener, thanks for your pull request. I think this is an interesting example and especially good for beginners. The code is very concise. I have a few comments.
| open Microsoft.Quantum.Arithmetic; | ||
|
|
||
| // This operation adds a (-1)-phase to the marked state(s) | ||
| operation ReflectMarked (inputQubits : Qubit[]) : Unit is Adj + Ctl { |
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.
You never call the adjoint or a controlled version of the function, so Adj + Ctl could be removed.
| // Flip the outputQubit for marked states. | ||
| // Here: For the state with alternating 0s and 1s | ||
| within { | ||
| ApplyToEachCA(X, inputQubits[0..2..Length(inputQubits)-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.
Considering to drop Adj + Ctl from this operation, this can become ApplyToEach
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.
Support for Adjoint is required for operations called inside a within block, but I agree that controlled shouldn't be required. It may be at the moment, though, as I'm not sure that microsoft/qsharp-compiler#152 is included in the 0.9 release.
| } | ||
|
|
||
| // This operation adds a (-1)-phase to the uniform superposition | ||
| operation ReflectUniform (inputQubits : Qubit[]) : Unit is Adj + Ctl { |
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.
See above.
| operation ReflectUniform (inputQubits : Qubit[]) : Unit is Adj + Ctl { | ||
| within { | ||
| // Transform the uniform superposition to all-zero | ||
| ApplyToEachCA(H, inputQubits); |
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.
This could be simplified to ApplyToEachA.
| return IntAsBoolArray(measuredInt, numQubits); | ||
| } | ||
| } | ||
| } No newline at end of file |
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.
Please add an empty line to the end of the file.
cgranade
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.
This looks really good, @thomashaener! I've left a few comments here and there, but thanks for adding this!
| ## Running the Sample in Visual Studio ## | ||
|
|
||
| Open the `QsharpSamples.sln` solution in Visual Studio and set the .csproj file in the manifest as the startup project. | ||
| Press Start in Visual Studio to run the sample. |
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 adding a host.py along with instructions on how to run the sample from the command line using either Python or the .NET Core SDK:
| Press Start in Visual Studio to run the sample. | |
| Press Start in Visual Studio to run the sample. | |
| ## Running the Sample from the Command Line ## | |
| This sample can also be run from the command line, including from the integrated terminal in Visual Studio Code. | |
| To run the C# host program for this sample, use the `dotnet` command: | |
| ```bash | |
| dotnet run | |
| ``` | |
| To run this sample from Python: | |
| ```bash | |
| python host.py | |
| ``` |
| // Flip the outputQubit for marked states. | ||
| // Here: For the state with alternating 0s and 1s | ||
| within { | ||
| ApplyToEachCA(X, inputQubits[0..2..Length(inputQubits)-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.
Support for Adjoint is required for operations called inside a within block, but I agree that controlled shouldn't be required. It may be at the moment, though, as I'm not sure that microsoft/qsharp-compiler#152 is included in the 0.9 release.
| apply { | ||
| // Flip the outputQubit for marked states. | ||
| // Here: For the state with alternating 0s and 1s | ||
| within { |
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.
Would it make sense to remove a level of nesting here?
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.
The outer level turns the bit-flip oracle into a phase-flip oracle, and the inner within-apply is the bit-flip oracle itself, so I left the two levels for illustration. Should I put an additional note there or similar?
| open Microsoft.Quantum.Arithmetic; | ||
|
|
||
| // This operation adds a (-1)-phase to the marked state(s) | ||
| operation ReflectMarked (inputQubits : Qubit[]) : Unit is Adj + Ctl { |
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.
| operation ReflectMarked (inputQubits : Qubit[]) : Unit is Adj + Ctl { | |
| operation ReflectAboutMarked(inputQubits : Qubit[]) : Unit is Adj + Ctl { |
| } | ||
|
|
||
| // This operation adds a (-1)-phase to the uniform superposition | ||
| operation ReflectUniform (inputQubits : Qubit[]) : Unit is Adj + Ctl { |
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.
| operation ReflectUniform (inputQubits : Qubit[]) : Unit is Adj + Ctl { | |
| operation ReflectAboutUniform(inputQubits : Qubit[]) : Unit is Adj + Ctl { |
| ReflectUniform(qubits); | ||
| } | ||
| // measure the answer | ||
| let measuredInt = MeasureInteger(LittleEndian(qubits)); |
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 feels a little funny to me to measure an integer, then immediately convert it to a Bool[]. Would it make sense to either return the Int directly, or to use return ForEach(MResetZ, qubits) here?
|
How does this sample relate to the Grover Kata, https://github.com/microsoft/QuantumKatas/tree/master/GroversAlgorithm? |
In my opinion, the purpose of the Katas is to teach programmers (in this particular case how to implement Grover), whereas samples illustrate how concise complete applications can be because of the language features that Q# provides (such as, e.g., within-apply in this sample). |
msoeken
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 for addressing all comments, just got one last small comment.
|
Thanks for adding this sample, @thomashaener! |
Added Validation to the Monomorphization transformation.
This PR adds a new sample that shows how to implement Grover's algorithm in Q#. The sample is supposed to be as simple as possible (e.g., for use in beginner tutorials).
The marking oracle uses a bit-flip oracle (as opposed to using a phase-flip directly) to better align with textbook descriptions and to make it easier to experiment with. E.g., one could combine it with the OracleSynthesis sample.