search

brayniac / histogram

rust histogram and percentile stats
Apache License 2.0
46 stars 4 forks source link

Return float results especially for stddev() #52

Closed kornysietsma closed 4 years ago

kornysietsma commented 4 years ago

I understand that all the values in the histogram are u64 - that makes sense - but why are the statistics all converted to u64 as well?

This isn't such a problem for percentiles (as they at least have the range of the original inputs) - but for standard deviation, it produces heavily truncated results if the input doesn't have a lot of variation.

It would be very nice to have f64 versions of these functions - I note that most of them are internally calculated as f64 and then converted to u64 on return.

(Actually I just discovered stddev is even worse! stdvar produces a f64 value, then truncates it to u64. And then stddev converts it back to a f64, takes the square-root, and truncates to u64 again)

brayniac commented 4 years ago

Oh, good point. I don't remember there being a real reason for returning u64 for stddev/stdvar/mean. I probably just did it to be consistent with the types returned for percentiles.

I'd be open to changing the API and releasing a new major version. It's important to remember that since the histogram stores counts for approximate values, we may still return values that don't seem right if we know the exact inputs.

If you submit a PR, I would be happy to review.