IQuantumProcessor interface #82
Conversation
src/Simulation/Simulators/QuantumExecutor/QuantumExecutorSimulator.cs
Outdated
Show resolved
Hide resolved
src/Simulation/Simulators/QuantumExecutor/QuantumExecutorSimulator.cs
Outdated
Show resolved
Hide resolved
src/Simulation/Simulators/QuantumExecutor/EmptyQuantumExecutor.cs
Outdated
Show resolved
Hide resolved
…/qsharp-runtime into alexva/QuantumExecutor
anpaz
left a comment
anpaz
left a comment
There was a problem hiding this comment.
I still have the concern that the ClassicallyControlled events receive an Action. the IQuantumProcessor is in principle an interface that receives notifications when quantum commands need to be executed/processed. The fact that it receives a notification of something that is not quantum kind of breaks this principle, but more over the fact that it receives an Action that should be executed to continue traversing the quantum commands makes it even worse.
A couple of different approaches have been suggested: in particular I think we could have one ContextChange method that gets invoked when the classical context changes (for example, because an if or a loop) passing a reason of what caused the context to change (based on the type of a Context parameter provided) would allow more flexibility and would allow us to have nested contexts simply by making the context the root of a linked-list. This might be something we want to consider before we're committed to the current approach.
| /// </summary> | ||
| /// <remarks> | ||
| /// To implement a target machine that executes quantum commands, implement this interface. | ||
| /// Consider using <see cref="EmptyQuantumProcessor"/> as a stub for easy impementation of this interface. |
There was a problem hiding this comment.
| /// Consider using <see cref="EmptyQuantumProcessor"/> as a stub for easy impementation of this interface. | |
| /// Consider using <see cref="EmptyQuantumProcessor"/> as a stub for easy implementation of this interface. | |
| ``` #Resolved |
| { | ||
| } | ||
|
|
||
| public virtual void ClassicallyControlled(Result measurementResult, Action onZero, Action onOne) |
There was a problem hiding this comment.
I wonder if it's better to simply mark all these methods as abstract instead of throwing not implemented at runtime. That way developers can use the compiler to identify what needs to be done. #ByDesign
There was a problem hiding this comment.
To use the compiler to identify what is not done, a developer would just implement IQuantumProcessor interface. That one is abstract, and will require implementing all the methods. The precise goal of this QuantumProcessorBase class is to allow a typical developer NOT to implement all functions, but instead just implement a few that are required. For example, Toffoli will just implement a handful of these, and let the unexpected ones cause an Exception.
In reply to: 350003808 [](ancestors = 350003808)
| /// A class that implements IQuantumProcessor that does not do any logic, but is convenient to inherit from. | ||
| /// It throws <see cref="NotImplementedException"/> for most APIs. | ||
| /// </summary> | ||
| public class EmptyQuantumProcessor : IQuantumProcessor |
There was a problem hiding this comment.
Maybe make this class abstract and rename this to AbstractQuantumProcessor? #ByDesign
There was a problem hiding this comment.
|
|
||
| /// <summary> | ||
| /// Called when controlled <a href="https://docs.microsoft.com/qsharp/api/qsharp/microsoft.quantum.intrinsic.y">Microsoft.Quantum.Intrinsic.Y</a> is called in Q#. | ||
| /// In Q# the operation applies X gate to <paramref name="qubit"/> controlled on <paramref name="controls"/>. The gate is given by matrix Y=((0,-𝑖),(𝑖,0)). |
There was a problem hiding this comment.
| /// In Q# the operation applies X gate to <paramref name="qubit"/> controlled on <paramref name="controls"/>. The gate is given by matrix Y=((0,-𝑖),(𝑖,0)). | |
| /// In Q# the operation applies Y gate to <paramref name="qubit"/> controlled on <paramref name="controls"/>. The gate is given by matrix Y=((0,-𝑖),(𝑖,0)). | |
| ``` #Resolved |
| { | ||
| /// <summary> | ||
| /// Called when <a href="https://docs.microsoft.com/qsharp/api/qsharp/microsoft.quantum.intrinsic.x">Microsoft.Quantum.Intrinsic.X</a> is called in Q#. | ||
| /// In Q# the operation applies X gate to <paramref name="qubit"/>. The gate is given by matrix X=((0,1),(1,0)). |
There was a problem hiding this comment.
| /// In Q# the operation applies X gate to <paramref name="qubit"/>. The gate is given by matrix X=((0,1),(1,0)). | |
| /// When this is invoked, it is expected that the X gate gets applied to the state of the given <paramref name="qubit"/>. The gate is given by matrix X=((0,1),(1,0)). |
I suggest all descriptions are modified from In Q# the operation applies... to a text similar to what I suggest above. #Resolved
| /// <param name="onOne">Corresponds to quantum program that must be executed if <paramref name="measurementResult"/> result is <see cref="ResultValue.One"/></param> | ||
| /// <remarks> | ||
| /// Calling <c>onZero()</c> will result in the execution of quantum program that Q# user intends to execute if <paramref name="measurementResult"/> result is <see cref="ResultValue.Zero"/>. | ||
| /// The program is executed with the same instance of <see cref="IQuantumProcessor"/> interface. |
There was a problem hiding this comment.
| /// The program is executed with the same instance of <see cref="IQuantumProcessor"/> interface. | |
| /// The program is executed within the same instance of <see cref="IQuantumProcessor"/> interface. | |
| ``` #Resolved |
| /// </remarks> | ||
| /// <typeparam name="T"></typeparam> | ||
| /// <param name="location">Provides information on where to generate the DumpMachine state.</param> | ||
| void OnDumpMachine<T>(T location); |
There was a problem hiding this comment.
why is it necessary to provide/implement OnDump handlers? #Resolved
There was a problem hiding this comment.
This is part of our standard simulator interface. Useful for debugging and for research on what the machine's state is. Not every physical machine will be able to dump, but some may, especially simulators.
In reply to: 351129532 [](ancestors = 351129532)
…/qsharp-runtime into alexva/QuantumExecutor
…/qsharp-runtime into alexva/QuantumExecutor
| /// </summary> | ||
| /// <param name="measurementResult">The actual result of the measurement of a qubit upon which branching is to be performed.</param> | ||
| /// <param name="resultValue">The expected value of result of the measurement of this qubit.</param> | ||
| /// <returns> A value representing this conditional statement and encoding the result of condition.</returns> |
There was a problem hiding this comment.
Maybe it is worth mentioning that the return value will be passed as a statement argument to RunThenClause, RepeatThenClause, RunElseClause, RepeatElseClause and EndConditionalStatement. I wonder where is the good place to desribe the whole mechanism together. #Resolved
There was a problem hiding this comment.
Minor: Is using long instead of int is slightly more future-proof? #Resolved
| /// Intended for a limited support of branching upon measurement results on a target machine level. | ||
| /// Called when the "then" statement of a conditional statement is about to be executed. | ||
| /// </summary> | ||
| /// <param name="statement">A value representing this conditional statement and encoding the result of condition.</param> |
There was a problem hiding this comment.
Maybe also mention here, that the value of statement parameter is the value returned by StartConditionalStatement. #Resolved
|
|
||
| /// <summary> | ||
| /// Intended for a limited support of branching upon measurement results on a target machine level. | ||
| /// Called when the "then" statement of a conditional statement has finished executing. |
There was a problem hiding this comment.
Just a clarification: RepeatThenClause will not be called if RunThenClause returned false. #ByDesign
There was a problem hiding this comment.
This is clearly explained in the comment to the RunThenClause. It says RepeatThenClause will not be called if RunThenClause returns false. RepeatThenClause also clearly states it will be called only after "Then" Clause has been actually executed.
In reply to: 357563259 [](ancestors = 357563259)
| /// Intended for a limited support of branching upon measurement results on a target machine level. | ||
| /// Called when the "else" statement of a conditional statement is about to be executed. | ||
| /// </summary> | ||
| /// <param name="statement">A value representing this conditional statement and encoding the result of condition.</param> |
There was a problem hiding this comment.
Maybe also mention here, that the value of statement parameter is the value returned by StartConditionalStatement. #Resolved
| /// Intended for a limited support of branching upon measurement results on a target machine level. | ||
| /// Called when the "else" statement of a conditional statement has finished executing. | ||
| /// </summary> | ||
| /// <param name="statement">A value representing this conditional statement and encoding the result of condition.</param> |
There was a problem hiding this comment.
Maybe also mention here, that the value of statement parameter is the value returned by StartConditionalStatement. #Resolved
| /// Intended for a limited support of branching upon measurement results on a target machine level. | ||
| /// Called when a conditional statement on measurement results has finished executing. | ||
| /// </summary> | ||
| /// <param name="statement">A value representing this conditional statement and encoding the result of condition.</param> |
There was a problem hiding this comment.
Maybe also mention here, that the value of statement parameter is the value returned by StartConditionalStatement. #Resolved
| { | ||
| public partial class QuantumProcessorDispatcher | ||
| { | ||
| private enum FunctorType |
There was a problem hiding this comment.
Similar enum is defined in https://github.com/microsoft/qsharp-runtime/blob/master/src/Simulation/Core/OperationFunctor.cs #Resolved
| /// </summary> | ||
| /// <param name="measurementResult">The actual result of the measurement of a qubit upon which branching is to be performed.</param> | ||
| /// <param name="resultValue">The expected value of result of the measurement of this qubit.</param> | ||
| /// <returns> A value representing this conditional statement and encoding the result of condition.</returns> |
There was a problem hiding this comment.
Minor: Is using long instead of int is slightly more future-proof? #Resolved
| : base(qubitManager ?? new QubitManagerTrackingScope(PreallocatedQubitCount, mayExtendCapacity:true, disableBorrowing:false)) | ||
| { | ||
| random = new System.Random(randomSeed == null ? DateTime.Now.Millisecond : randomSeed.Value); | ||
| QuantumProcessor = quantumProcessor ?? new QuantumProcessorBase(); |
There was a problem hiding this comment.
Do we really need an instance of a QuantumProcessor if none is given? We should keep it as null... #Resolved
There was a problem hiding this comment.
I see your point. But it does not make sense to have a dispatcher without Quantum processor. I would rather disallow passing in null. But I plan to revisit this place a little later anyway when we enable multiple processors working together, and this will change anyway.
In reply to: 353878732 [](ancestors = 353878732)
IQuantumExecutorinterface allows for easy development of simulators and executors of quantum commands.EmptyQuantumExecutorclass can be used to inherit from it for even easier implementation.