Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@guenp
Copy link

@guenp guenp commented Jul 6, 2020

Initial new version of RUS sample that breaks up circuit into two parts (red and blue, see image) and only runs second (blue) part if ancilla measurement in the first part (red) returns Zero.
Draft version todo's
[x] docs
[x] unit tests
[o] add hardware-compatible sample -> moving this to follow up deliverable (TBD)

@guenp guenp requested review from apaetz, cgranade, msoeken and swernli July 6, 2020 17:17
Copy link
Contributor

@cgranade cgranade left a 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 really good so far! I've made some initial suggestions, but I think once those are addressed, and once a README.md is added, this should be in good shape!

@swernli
Copy link
Contributor

swernli commented Jul 6, 2020

This seems like an excellent sample for demonstrating the algorithm on Q#, but I don't think this would work as written when targeting partner execution due to some translation limitations. Is the goal here to produce a sample that works across both simulation and execution or is this more focused on a sample of clean, efficient Q#? I just want to make sure I understand the goal before I get too deep into feedback along the wrong axis :).

@guenp
Copy link
Author

guenp commented Jul 6, 2020

This seems like an excellent sample for demonstrating the algorithm on Q#, but I don't think this would work as written when targeting partner execution due to some translation limitations. Is the goal here to produce a sample that works across both simulation and execution or is this more focused on a sample of clean, efficient Q#? I just want to make sure I understand the goal before I get too deep into feedback along the wrong axis :).

@swernli Is there documentation anywhere on these translation limitations? I suggest we could make two versions.

@swernli
Copy link
Contributor

swernli commented Jul 6, 2020

We don't have the documentation yet, though there is work ongoing to capture the supported features as quantum processor capabilities (see microsoft/qsharp-compiler#488), which would allow developers to get compile time feedback on whether their code would work as-written for a particular target. As for two samples vs one, I'll let @cgranade weigh in there. I think the samples included here have thus far been primarily for pure Q#, but I don't know if there is any expectation around shifting that focus to also include hardware compatible samples.

@guenp
Copy link
Author

guenp commented Jul 6, 2020

We don't have the documentation yet, though there is work ongoing to capture the supported features as quantum processor capabilities (see microsoft/qsharp-compiler#488), which would allow developers to get compile time feedback on whether their code would work as-written for a particular target. As for two samples vs one, I'll let @cgranade weigh in there. I think the samples included here have thus far been primarily for pure Q#, but I don't know if there is any expectation around shifting that focus to also include hardware compatible samples.

@cgranade I think it would be useful to have hardware-compatible examples in the samples for those running stuff on Azure quantum. My suggestion would be to make two versions and have an input argument --hw-compatible or so. Any thoughts?

@cgranade
Copy link
Contributor

cgranade commented Jul 6, 2020

We don't have the documentation yet, though there is work ongoing to capture the supported features as quantum processor capabilities (see microsoft/qsharp-compiler#488), which would allow developers to get compile time feedback on whether their code would work as-written for a particular target. As for two samples vs one, I'll let @cgranade weigh in there. I think the samples included here have thus far been primarily for pure Q#, but I don't know if there is any expectation around shifting that focus to also include hardware compatible samples.

I think a key principle needs to be that adapting samples to be hardware-compatible shouldn't make it harder to understand the samples or what they teach about Q# and the Quantum Development Kit. Thus, I'm all for adapting existing samples to be compatible with current capabilities if we can do so in a fairly nice way, otherwise I would suggest splitting into two samples.

We have some precedent for this, in fact, with the host-language interoperability samples. The Python interoperability feature has some limitations that make it hard to use for all existing samples, such that there's a dedicated samples section under interoperability/python.

@cgranade I think it would be useful to have hardware-compatible examples in the samples for those running stuff on Azure quantum. My suggestion would be to make two versions and have an input argument --hw-compatible or so. Any thoughts?

Agreed with making two versions, as per the above. I don't know that we could make them switch with an input argument, though, given that Q# applications compile as whole programs; @swernli could likely clarify here.

@swernli
Copy link
Contributor

swernli commented Jul 6, 2020

@cgranade That all sounds reasonable to me.

@guenp Given that we plan on having a separate sample for hardware targets, I don't think anything here needs to be adjusted for translation compatibility.

I can tell it's a great sample, because I've already learned a few new tricks from it! Well done!

Copy link
Member

@msoeken msoeken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the sample @guenp. I left a few comments.

Guen P and others added 2 commits July 8, 2020 07:33
Guen Prawiroatmodjo added 2 commits July 9, 2020 10:41
…te, update docstrings, add InitializeQubits operation to replace PrepareValueForBasis
@guenp
Copy link
Author

guenp commented Jul 9, 2020

@cgranade That all sounds reasonable to me.

@guenp Given that we plan on having a separate sample for hardware targets, I don't think anything here needs to be adjusted for translation compatibility.

I can tell it's a great sample, because I've already learned a few new tricks from it! Well done!

Thanks @swernli! We discussed during scrum and we decided to wait with implementing a hardware-compatible sample in our public repo until we go live with the Azure Quantum public preview. I'll follow up with you on guidance for this when that deliverable gets closer :)

@guenp guenp marked this pull request as ready for review July 10, 2020 16:18
Copy link
Member

@msoeken msoeken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks great! Had just one small comment.

Guen Prawiroatmodjo and others added 3 commits July 10, 2020 09:26
Change placement of parens for 'done' logic

Co-authored-by: Mathias Soeken <mathias.soeken@outlook.com>
Copy link
Contributor

@cgranade cgranade left a 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 really good, @guenp! Just a few more suggestions that I had to tie things up. Thank you!

@guenp guenp force-pushed the guen/samples-12394-rus branch from 979b7e5 to af7ec7e Compare July 13, 2020 17:16
Copy link
Contributor

@cgranade cgranade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is almost there (sorry for as many rounds...!), just a couple last comments. I've marked as approved, as these are all suggestions. Thank you!

Guen P and others added 2 commits July 13, 2020 11:47
Improvements to docs, use unicode instead of latex

Co-authored-by: Chris Granade <chgranad@microsoft.com>
@guenp guenp merged commit 1ac11dd into master Jul 13, 2020
@guenp guenp deleted the guen/samples-12394-rus branch July 13, 2020 19:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants