Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

task 1.7 global phase change was added #177

Merged
merged 15 commits into from
Oct 5, 2019

Conversation

adutchengineer
Copy link
Contributor

No description provided.

@msftclas
Copy link

msftclas commented Oct 2, 2019

CLA assistant check
All CLA requirements met.

Copy link
Member

@tcNickolas tcNickolas left a comment

Choose a reason for hiding this comment

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

Thank you for contributing this - manipulating global phase shows up in the later katas a lot as part of more complicated tasks, so it is something we need to introduce early! It might make sense to add a comment to that effect as a note - something along the lines of "Note: this change on its own is not observable - there is no experiment you can do on a standalone qubit to figure out whether it acquired the global phase or not. However, you can use a controlled version of this operation to observe the global phase it introduces. This is used in later katas as part of more complicated tasks."

This change is a good start; I did a review pass and left some small comments.

Thank you!

BasicGates/Tests.qs Outdated Show resolved Hide resolved
BasicGates/Tests.qs Outdated Show resolved Hide resolved
BasicGates/Tests.qs Outdated Show resolved Hide resolved
BasicGates/Tests.qs Outdated Show resolved Hide resolved
BasicGates/Tasks.qs Outdated Show resolved Hide resolved
BasicGates/BasicGates.ipynb Outdated Show resolved Hide resolved
@tcNickolas
Copy link
Member

(I also took a look at the CI failure - it is a transient error I've been seeing sometimes, nothing to worry about in relation to your change)

adutchengineer and others added 6 commits October 2, 2019 20:31
Co-Authored-By: Mariia Mykhailova <michaylova@gmail.com>
Co-Authored-By: Mariia Mykhailova <michaylova@gmail.com>
Co-Authored-By: Mariia Mykhailova <michaylova@gmail.com>
Co-Authored-By: Mariia Mykhailova <michaylova@gmail.com>
Copy link
Contributor Author

@adutchengineer adutchengineer 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 I made the appropriate adjustments. Let me know if I missed something.

BasicGates/Tasks.qs Outdated Show resolved Hide resolved
Copy link
Member

@tcNickolas tcNickolas left a comment

Choose a reason for hiding this comment

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

The task numbers themselves were fine, it was just the names of the tests that needed to be changed. I feel bad that you did all these changes that were not needed, so I went in and undid the changes to task numbering, and did a bit of cleanup while I was at it. This change should be good now - once CI passes, I'll merge it.

Thank you!

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.

3 participants