tisonkun commented on PR #23: URL: https://github.com/apache/datasketches-rust/pull/23#issuecomment-3661234667
The remaining task in my mind: * Implement cdf/pmf functions * Support reading format of the reference implementation and snapshot tests * Figure out whether to cover NaN edge cases > > Shall we support tdigest over f32? Java implements only TDigestDouble while C++ has both for float and double. I think f64 is enough to cover most cases. > > Is there any way we can have a `TDigest<T: num::Float>`? I think in some cases having less precision for half of the memory is desirable. I may not include this feature in this PR, because: 1. The Java impl has only TDigestDouble. 2. Generally, f64 is what you need. 3. Switching between f32 and f64 can increase a lot of type tuning issues and perhaps abstractions only for making this work. That said, I'll try to see if the `weight` field can be a `f64` so we don't need a type config to associate (f64, u64)/(f32, u32) anyway. But the Java/C++ impl does use an integer type for weight, while using f64 for u64 should be lossless since the only assignment here is: ```rust self.weight = total_weight; let total_weight = self.weight + other.weight; ``` cc @leerho @AlexanderSaydakov - any reason we must use an integer for `weight`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
