-
Notifications
You must be signed in to change notification settings - Fork 2
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
Optional rounding of XYShape #47
Comments
Honestly, I wouldn't worry about this. AFAIK JS engines use 32 bits for floats across the board, and so does PostgreSQL's |
I think it would make a difference for graphql/api. Sending distributions over client-server now does take a fair amount of time. I imagine that reducing this could reduce that by 1/2 or less. My impression is that when it's sent as a GraphQL param, it encodes it in JSON, and sent as basically a string, though I'm not sure. It's also something we could test experimentally, maybe with a simpler system (like, something that just trunctates after 2 decimal places) |
Okay, in that case I think an easier solution is to pack the numbers into a Float32Array, and send that over the wire as UTF-8 (via TextEncoder/TextDecoder browser API). That should be quite fast. |
Huh, that seems neat, it could be a really good fit. |
Right now distributions in the dist builder result in a lot of unnecessary precision. For instance,
This takes up a lot of extra space and doesn't add anything really. There is a way on the server to request rounding, but it probably doesn't currently work because it's not supported by the DistPlus ReasonML library.
A crude method would be something like, "use n digits of precision for each number". One downside to this that it's possible that the entire distribution may be between very precise numbers, like 3.48382 to 3.48398.
Ideally there would be some way of approximating how many digits of precision is reasonable to use, or having different floats use different levels of precision.
Note that these values are sent via the GraphQL API and saved to the database, so shortness would likely help a fair bit. It also takes time to render them; I imagine improvements here could speed things up a fair bit.
The text was updated successfully, but these errors were encountered: