Skip to content
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

Fixed point decimal representation. #464

Closed
cgyurgyik opened this issue Mar 27, 2021 · 2 comments · Fixed by #468
Closed

Fixed point decimal representation. #464

cgyurgyik opened this issue Mar 27, 2021 · 2 comments · Fixed by #468
Labels
C: fud Calyx Driver S: Discussion needed Issues blocked on discussion

Comments

@cgyurgyik
Copy link
Collaborator

cgyurgyik commented Mar 27, 2021

Testing my recent changes a bit more thoroughly, I believe there are two ways this can go.

(1) The user passes the following:

{
  ... 
  data: [ 0.1 ]
}

The value pulled from the JSON key data is now: 0.1000000000000000055511151231257827021181583404541015625. This value is indeed representable by fixed point, given enough fractional bits, so the error they receive is fractional width overflow, required fractional width: 55..

(2) The user passes in the data as strings, e.g.

{
  ... 
  data: [ "0.1" ]
}

This ensures that 0.1 indeed has the value 0.1, and thus the error they receive is not representable by fixed point numbers: 0.1.

To me, (2) seems less surprising to the user, and we can easily ensure that strings are actually passed for fixed point. Option (3) might be to explore other JSON libraries for Python that gives us more control over how numbers are interpreted.

@cgyurgyik cgyurgyik added S: Discussion needed Issues blocked on discussion C: fud Calyx Driver labels Mar 27, 2021
@cgyurgyik
Copy link
Collaborator Author

I'm going to spend some time boxing up Fixed Point number and Bitnum number representations in respective Python classes to make it a bit more clean and extensible. For simplicity, I'll just assume you pass in a string version of the number to the class.

@cgyurgyik
Copy link
Collaborator Author

Looks like simplejson provides this option. We can avoid the cast to float and directly convert it to a string instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: fud Calyx Driver S: Discussion needed Issues blocked on discussion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant