-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add vectorized from_bits
and to_bits
#1199
Add vectorized from_bits
and to_bits
#1199
Conversation
@@ -528,6 +592,13 @@ def from_bits(self, bits: Sequence[int]) -> Fxp: | |||
fxp_bin = "0b" + bits_bin[: -self.num_frac] + "." + bits_bin[-self.num_frac :] | |||
return Fxp(fxp_bin, dtype=self.fxp_dtype_str) | |||
|
|||
def from_bits_array(self, bits_array: NDArray[np.uint8]): | |||
assert isinstance(self.bitsize, int), "cannot convert to bits for symbolic bitsize" | |||
# TODO figure out why `np.vectorize` is not working here |
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.
Add a link to an open issue with the TODO?
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'm planning to remove the fxpmath
dependency in classical sim for now, in a follow-up PR, so this might become irrelevant. I can perhaps open an issue later when we decide to actually use fxpmath as our floating point library.
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.
You want to remove all code that uses fxpmath
and remove the QFxp
type ? That seems aggressive.
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.
No no, the QFxp
type will stay, but will use integer values (i.e. binary repr of the fxp) for the classical simulator (i.e. from_bits and to_bits), instead of Fxp
.
And other uses of fxpmath
in the code will not be touched, e.g. the current phase gradient scaled_val
etc.
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.
So when calling bloq.call_classically(phase_grad=_fxp_value_)
; the _fxp_value_
would be an int I believe ?
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.
Yep, like it is right now. I'll add a helper method to QFxp
to convert a float/Fxp value into the correct int value for passing to the classical sim, so that end users don't have to manually figure out the correct binary representation.
Adds
from_bits_array
andto_bits_array
, and uses these functions in classical_sim'sints_to_bits
andbits_to_ints
(which will be removed in a follow up PR).part of #1142