tisonkun commented on issue #157:
URL:
https://github.com/apache/datasketches-rust/issues/157#issuecomment-5048839263
> In your Rust implementation you suggested using "u8" to hold the exponent,
which can be as large as 1023. Unsigned 8 bits has a max of 255. I suggest at
least a u16 for this value.
Could you tell a real world execution situation that the input value can be
larger than 255? In the lookup table from datasketches-cpp, it holds for the
first 0-255 values and in all the use case in datasketches-rust, the input
value is always u8:
```rust
impl CpcSketch {
fn update_hip(&mut self, row_col: u32) {
let k = 1 << self.lg_k;
let col = (row_col & 63) as usize; // <-- col + 1 <= 64
let one_over_p = (k as f64) / self.kxp;
self.hip_est_accum += one_over_p;
self.kxp -= INVERSE_POWERS_OF_2[col + 1] // notice the "+1"
}
fn refresh_kxp(&mut self, bit_matrix: &[u64]) {
for i in (0..8).rev() {
// the reverse order is important
let factor = INVERSE_POWERS_OF_2[i * 8]; // pow (256.0, (-1.0 *
((double) j))) <-- i * 8 <= 64
total += factor * byte_sums[i];
}
// ...
}
}
impl HipEstimator {
fn update_kxq(&mut self, old_value: u8, new_value: u8) { // <-- both
old_value and new_value are inherited u8
// Subtract old value contribution
if old_value < 32 {
self.kxq0 -= INVERSE_POWERS_OF_2[old_value as usize];
} else {
self.kxq1 -= INVERSE_POWERS_OF_2[old_value as usize];
}
// Add new value contribution
if new_value < 32 {
self.kxq0 += INVERSE_POWERS_OF_2[new_value as usize];
} else {
self.kxq1 += INVERSE_POWERS_OF_2[new_value as usize];
}
}
}
```
Besides, while we have a benchmark result that in an isolated `inv_pow2`
loop, the bit computation is faster than a lookup table, our concrete
calculations of `INVERSE_POWERS_OF_2` do not fall into that scenario heavily.
But as it does no harm and reduces read-only data, we may still refactor to use
the bit computation version.
--
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]
