Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UnitaryPatterns] Workbook Solutions Complete #377

Merged
merged 49 commits into from
Jul 12, 2020

Conversation

jainvasu631
Copy link
Contributor

@jainvasu631 jainvasu631 commented Jun 15, 2020

Added Workbook_UnitaryPatterns and cross-linked it with UnitaryPatterns as discussed in Comment in Issue #208.

The Workbook Solutions for all Tasks from 1 to 15 have been done.
I didn't notice the solutions in ReferenceImplementation.qs hence I have implemented my own solutions for the most part. They are mostly similar to the ReferenceImplementations.
For Task 13 - Removed Encapsulated Functions since that was causing errors in the Kernel Cell
For Task 14 - The Solution is longer but I believe it is easier to comprehend that the reference solution.
For Task 15 - I have simplified the solution. The reference solution didn't take advantage of the fact that both indices are always consecutive.

All checks are now passing.

@jainvasu631 jainvasu631 marked this pull request as ready for review June 15, 2020 14:21
@jainvasu631 jainvasu631 changed the title [UnitaryPatterns] Workbook Solutions for Task 1-13 [UnitaryPatterns] Workbook Solutions Complete Jun 16, 2020
@jainvasu631 jainvasu631 marked this pull request as draft June 16, 2020 18:04
@jainvasu631 jainvasu631 marked this pull request as ready for review June 16, 2020 20:35
@tcNickolas
Copy link
Member

Could you please unentangle this PR from the MultiQubitGates change in #375? Looks like you've branched this PR off that branch, and I'd prefer to review them separately, since they are independent changes.

@jainvasu631
Copy link
Contributor Author

Apologies. I didnt realise those changes got entangled with this Branch. I have reset the file to MultiQubitGates.ipynb as it is on the latest commit in Microsoft/QuantumKatas

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.

I did a round of cleanup - it looks like a lot of changes but in fact it's mostly spelling, capitalization and phrasings. The larger changes are:

  • I added an image for a decrement circuit (I think it's more descriptive than just saying "a cascade of gates")
  • I removed applications of I gates - it's not necessary, and I'd rather not teach the learner to apply identity when it's not necessary, this causes all kinds of weird code patterns later :-)
  • In task 15 T_N(i) can be formatted a lot neater using \ddots command, and I think there were typos in spelling out T_2(1) and T_2(2) - the rows of 2x2 blocks were shifted by one element.

Looks great, and it's a very serious amount of work - it took me a a fair amount of time to go though it all (admittedly, mostly because I'm not very familiar with the last couple of tasks).

Thank you!

@tcNickolas
Copy link
Member

Looking at your initial comment, what do you mean by "Encapsulated Functions" and what kinds of errors were they causing?

@tcNickolas tcNickolas merged commit 2e16356 into microsoft:master Jul 12, 2020
@jainvasu631
Copy link
Contributor Author

While writing solutions for Task 13, I used the helper operation Decrement to refactor some code. However, since this name was used in Reference Implementation.qs I kept getting errors. Finally I decided to not refactor the code and instead wrote the entire function in main body only. Hence I was probably referring to the fact that operations in ReferenceImplementation.qs cause namespace conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants