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

Feature request: support floating-point numbers #128

Open
justinlovinger opened this issue Sep 23, 2023 · 5 comments
Open

Feature request: support floating-point numbers #128

justinlovinger opened this issue Sep 23, 2023 · 5 comments

Comments

@justinlovinger
Copy link

Feature request: support floating-point numbers

See #91,

Floating point values can be supported, but they are basically a completely separate implementation that wraps an inner integer-based histogram. I'd be neat to port DoubleHistogram to Rust as well, but unfortunately it's a larger effort that I don't have time for myself at the moment. If you want to give it a try, I'd be happy to try and review? It should be a fairly straightforward translation of the Java version I linked above! I wonder if we may even be able to have a single Histogram type by providing inherent implementations for Histogram<f64>... Not entirely clear. Initially I'd just have a separate DoubleHistogram type.

If possible, I suggest implementing a type-generic histogram using num-traits.

@jonhoo
Copy link
Collaborator

jonhoo commented Sep 23, 2023

It won't be possible to use num-trait here I don't think. Floats have to be handled fundamentally differently in HDR histograms is my understanding.

@justinlovinger
Copy link
Author

But could all floats be handled with the same implementation? I meant something like RealHistogram<A: Real> or FloatHistogram<A: Float>.

Unfortunately, we cannot do the same for integer histograms because num-traits lacks an "integer but not real" trait to differentiate numbers with whole values from numbers with whole values and decimals.

@jonhoo
Copy link
Collaborator

jonhoo commented Sep 23, 2023

Probably, though we could do that with a trait of our own that we implement just for f32 and f64 rather than going via num-traits. That way we're also reducing the breaking change hazard. Alternatively just implement for f64.

@justinlovinger
Copy link
Author

If we implement our own trait, it will not work with other types implementing num-traits traits. For example, the application I want to use HdrHistogram for has a type wrapping f64. num-traits is already used extensively in the Rust ecosystem.

@jonhoo
Copy link
Collaborator

jonhoo commented Sep 24, 2023

Yup, well aware of the implications. I'd want to see how the implementation ultimately pans out — I suspect going via num-traits might end up quite painful (vs just supporting f64), but happy to be proven wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants