-
Notifications
You must be signed in to change notification settings - Fork 920
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.
I think this looks like a good addition, but my main concern is that it currently reads more as a library than as a sample — it'd be great if we could add more comments to help the reader understand the new operations and functions added with this PR, and about how they differ from what is currently in the library. It'd also be great to have a libraries issue we can track for adding this functionality so that we know when to remove the new functions and operations from this sample in favor of calling in to the libraries directly.
Here is the corresponding issue in the library: microsoft/QuantumLibraries#423 Once this is approved we can use a lot of operations from there. |
Co-authored-by: Cassandra Granade <[email protected]>
Co-authored-by: Cassandra Granade <[email protected]>
@cgranade I think I caught all spelling cases. The Markdown check complains about broken links in the README file that are in the manifest and point to locations which will only be available after the merge. What is the guidance here? |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -19,7 +19,8 @@ | |||
"languageId": ["qsharp"], |
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 adding these!
Co-authored-by: Cassandra Granade <[email protected]>
This allows to use the sparse simulator with the factoring sample. It will be used as default simulator, but the full-state simulator can be activated using a flag. The PR also contains various arithmetic operations, which can eventually be merged into the quantum libraries after API review.