-
Notifications
You must be signed in to change notification settings - Fork 16
Add magic rounding support for (1,1,p) and (2,1,p) QRACs #33
Conversation
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.
This is great, thanks!
My only additional comment is that I think vars_per_qubit
should be changed to _vars_per_qubit
everywhere within RoundingContext
(it can remain as it is, however, in magic_rounding.py
), both as an argument name in the constructor and in its name as assigned on the object. It really only exists to make our lives easier in test_magic_rounding.py
and will be dropped as soon as we refactor to have adaptive QRACs. Since we don't plan to support this new API indefinitely, let's introduce it as protected instead of public (i.e., with the leading underscore). The only files that should need modified as part of this tweak are rounding_common.py
and test_magic_rounding.py
.
I added tests in 2a9e176. There's no explicit test of weighted sampling for (2,1,p) and (1,1,1) QRACs -- at least, not yet. I think that's the only thing missing (and I don't think I'd hold up merging to wait on it). |
Co-authored-by: Jim Garrison <jim@garrison.cc>
Co-authored-by: Jim Garrison <jim@garrison.cc>
Summary
Details and comments