-
Notifications
You must be signed in to change notification settings - Fork 207
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
Bosonic logarithmic mapper #1356
Conversation
Pull Request Test Coverage Report for Build 8735231706Details
💛 - Coveralls |
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.
I focused on design and docs in this review and did not check the derivations or math.
I left some comments and once addressed this will fit nicely with the other mappers.
releasenotes/notes/bosonic-logarithmic-mapper-4b1f24c4ca16cf8b.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/bosonic-logarithmic-mapper-4b1f24c4ca16cf8b.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/bosonic-logarithmic-mapper-4b1f24c4ca16cf8b.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Max Rossmannek <[email protected]>
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.
Some phrasing and one more refactoring idea
releasenotes/notes/bosonic-logarithmic-mapper-4b1f24c4ca16cf8b.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Max Rossmannek <[email protected]>
Co-authored-by: Max Rossmannek <[email protected]>
Co-authored-by: Max Rossmannek <[email protected]>
679f60e
to
5004530
Compare
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.
Okay, last round. I took a look at the rendered docs and found some more things that need fixing. See comments below.
Finally, I found this one odd break
statement which we should address. After that this is ready to go 👍
releasenotes/notes/bosonic-logarithmic-mapper-4b1f24c4ca16cf8b.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/bosonic-logarithmic-mapper-4b1f24c4ca16cf8b.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/bosonic-logarithmic-mapper-4b1f24c4ca16cf8b.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Max Rossmannek <[email protected]>
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.
Thanks! A minor detail to comment on but that's it 🙂
I was wondering whether we should highlight the new ValueError
in the release note, but I honestly don't think it is necessary.
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.
Thanks a lot, @ftroisi! Great work 👍
Summary
This MR introduces the Bosonic Logarithmic Mapper, which allows to map the BosonicOp class to the qubit space using binary encoding.
It addresses Issue #1345
Details and comments
See the issue above for more details on the reference papers