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

Devikamehra/improve error join measurement #453

Conversation

devikamehra
Copy link
Contributor

#176 Improved error statements in Joint Measurement Kata.

@devikamehra devikamehra marked this pull request as ready for review August 5, 2020 13:53
@devikamehra devikamehra marked this pull request as draft August 5, 2020 14:19
@tcNickolas tcNickolas changed the base branch from OneWeekHackathon to master August 10, 2020 19:10
Copy link
Contributor

@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.

That's a good start!

I'd take this change one step further and add the same level of details that Measurements kata offers: Misclassified |1⟩ as |0⟩ in 52 test runs. (having the string representations of the states misclassified, and the ability to report how many times each state was misclassified as one or another error) - it is a lot easier for the learner to troubleshoot the states without being exposed to the internal indices we use to mark them. You'd have to pass the string representations as a String[] array as an argument, and to track a 2-dimensional array of errors (written is a 1-dimensional for convenience).

Thank you!

@devikamehra devikamehra marked this pull request as ready for review September 2, 2020 17:54
@devikamehra devikamehra changed the base branch from master to OneWeekHackathon September 2, 2020 18:03
@devikamehra devikamehra changed the base branch from OneWeekHackathon to master September 2, 2020 18:36
Copy link
Contributor

@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 made a pass and left some comments - I think I found the cause of CI build break, so once you fix those, it should work.

Thank you!

Copy link
Contributor

@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.

Looks good! I reverted part of the change that reintroduced deprecated functions RandomInt and RandomReal - they've been deprecated in the most recent QDK release and removed in ebc0ce0 - and some whitespace changes - those were all minor and didn't warrant another review iteration.

Thank you!

@tcNickolas tcNickolas merged commit c53f17e into microsoft:master Sep 5, 2020
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