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
[Code together] Add matrix form to Grover Operator #1553
Conversation
…ion of Observable
In case you prefer the first one, I'll remove the new private function |
Hi @ingstra, thank you for the submission! 🙂 No clear preference for the approach, other than it producing the correct matrix representation. Overall, the more performant approach would be preferable, though this is not required for us to accept the solution. It could be nice to go with one of them and add further parts of the PR (e.g., testing). Also, don't forget to update the changelog with an entry about the addition and by adding yourself to the list of contributors. 🙂 |
Codecov Report
@@ Coverage Diff @@
## master #1553 +/- ##
=======================================
Coverage 99.15% 99.15%
=======================================
Files 189 189
Lines 13417 13431 +14
=======================================
+ Hits 13303 13317 +14
Misses 114 114
Continue to review full report at Codecov.
|
Ok! Where in the changelog should I write? Top? Bottom? |
Sorry for the billion commits, it's my first time... |
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>
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.
Hi @ingstra, this is looking nice! 🙂 Overall, it's already in a good shape. Left some suggestions, feel free to address those that were marked as minor last.
Also, there seems to be a conflict in the changelog. This can be resolved by pulling master (git checkout master
followed by git pull
), switching to the feature branch (git checkout add_reseterror_support
) and then merging master
(git merge master
). The culprit seems to be the changelog file. We'd like to have all the entries kept there, for some reason git
could not resolve the addition there well.
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>
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.
Thank you so much for this contribution @ingstra! 🎉 🙂
|
||
@property | ||
def matrix(self): | ||
# Redefine the property here to allow for a custom _matrix signature |
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.
Nice to have this comment!
# s1 = H|0>, Hadamard on a single qubit in the ground state | ||
s1 = np.array([1, 1]) / np.sqrt(2) |
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.
Nice!
Issue #1514
Add matrix attribute to Grover template.