Fixed a few errors and sped up estimation by a large factor

[LucaCappelletti94]

Hi @jedisct1 - I was benchmarking this implementation of HyperLogLog alongside a few others, and I noticed a few significant errors that I thought best to fix. Here is what I changed:

1. Fixed indexing error for linear count threshold - before you had forgotten the -4 there, the precision exponent and the threshold were off.

rust-hyperloglog/src/lib.rs

Lines 132 to 134 in bf12a8a

	fn get_threshold(p: u8) -> f64 {
	THRESHOLD_DATA[p as usize - 4]
	}

2. I have fixed the unordered estimates, aligned with the biases

rust-hyperloglog/src/weights.rs

Lines 4291 to 4313 in f190a71

	#[test]
	fn test_threshold_data_is_sorted() {
	for i in 1..THRESHOLD_DATA.len() {
	assert!(THRESHOLD_DATA[i - 1] < THRESHOLD_DATA[i]);
	}
	}
	#[test]
	fn test_raw_estimate_data_is_sorted() {
	for i in 1..RAW_ESTIMATE_DATA.len() {
	for j in 1..RAW_ESTIMATE_DATA[i].len() {
	assert!(RAW_ESTIMATE_DATA[i][j - 1] < RAW_ESTIMATE_DATA[i][j], "precision: {}, value: {}", i + 4, j);
	}
	}
	}
	#[test]
	fn test_estimate_bias_length() {
	assert_eq!(BIAS_DATA.len(), 15);
	for i in 0..BIAS_DATA.len() {
	assert_eq!(BIAS_DATA[i].len(), RAW_ESTIMATE_DATA[i].len());
	}
	}

3. Given now that the estimates are sorted, we can use a partition point search to find the neighbours.

rust-hyperloglog/src/lib.rs

Lines 157 to 181 in f190a71

	fn estimate_bias(estimate: f64, p: u8) -> f64 {
	let bias_vector = BIAS_DATA[(p - 4) as usize];
	let estimate_vector = RAW_ESTIMATE_DATA[(p - 4) as usize];
	// Since the estimates are sorted, we can use a partition point to find the nearest neighbors
	let partition_point = estimate_vector.partition_point(|&x| x < estimate);
	let mut min = if partition_point > 6 {
	partition_point - 6
	} else {
	0
	};
	let mut max = core::cmp::min(partition_point + 6, estimate_vector.len());
	while max - min != 6 {
	let (min_val, max_val) = (estimate_vector[min], estimate_vector[max - 1]);
	if 2.0 * estimate - min_val > max_val {
	min += 1;
	} else {
	max -= 1;
	}
	}
	(min..max).map(|i| bias_vector[i]).sum::<f64>() / 6.0
	}

Other changes are mostly aesthetic to make the code, in my opinion, more readable.

Do note that this pull request needs for the siphash pull request to be merged

Cheers!

[finnbear]

Are you sure this is correct? I'm under the impression that SipHasher13 maintains state that isn't cleared by finish, meaning that future identical value's will hash differently.

[jedisct1]

I think let mut sip = self.sip; implicitly clones the value.

[finnbear]

Ah, right, it implements Copy.

[LucaCappelletti94]

Yeah, although I think replacing it with a generic that implements the Hasher trait would be cleaner.