From 3b3e26c4b8b2a4e67645c2038dae93d50f280564 Mon Sep 17 00:00:00 2001 From: Pascal Seitz Date: Tue, 8 Jun 2021 13:02:42 +0200 Subject: [PATCH] use f64 precision for slope calculation --- fastfield_codecs/src/linearinterpol.rs | 6 ++++-- src/common/mod.rs | 4 ---- src/fastfield/reader.rs | 11 ++++------- src/fastfield/serializer/mod.rs | 1 - 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/fastfield_codecs/src/linearinterpol.rs b/fastfield_codecs/src/linearinterpol.rs index b3aea0de3..1bf7c8b9c 100644 --- a/fastfield_codecs/src/linearinterpol.rs +++ b/fastfield_codecs/src/linearinterpol.rs @@ -152,7 +152,9 @@ impl LinearInterpolFastFieldSerializer { } } fn get_slope(first_val: u64, last_val: u64, num_vals: u64) -> f32 { - (last_val as f32 - first_val as f32) / (num_vals as u64 - 1) as f32 + // We calculate the slope with f64 high precision and use the result in lower precision f32 + // This is done in order to handle estimation for very large values like i64::MAX + ((last_val as f64 - first_val as f64) / (num_vals as u64 - 1) as f64) as f32 } fn get_calculated_value(first_val: u64, pos: u64, slope: f32) -> u64 { @@ -163,7 +165,7 @@ impl FastFieldSerializerEstimate for LinearInterpolFastFieldSerializer { /// where the local maxima are for the deviation of the calculated value and /// the offset is also unknown. fn estimate(fastfield_accessor: &impl FastFieldDataAccess, stats: FastFieldStats) -> f32 { - if stats.max_value > i64::MAX as u64 / 2 || stats.num_vals < 3 { + if (stats.max_value - stats.min_value) > i64::MAX as u64 / 2 || stats.num_vals < 3 { return f32::MAX; //disable compressor for this case } let first_val = fastfield_accessor.get(0); diff --git a/src/common/mod.rs b/src/common/mod.rs index 4e574f1a0..29a4b057c 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -1,14 +1,11 @@ mod bitset; mod composite_file; mod counting_writer; -//mod serialize; -//mod vint; pub use self::bitset::BitSet; pub(crate) use self::bitset::TinySet; pub(crate) use self::composite_file::{CompositeFile, CompositeWrite}; pub use self::counting_writer::CountingWriter; -//pub use self::serialize::{BinarySerializable, DeserializeFrom, FixedSize}; pub use byteorder::LittleEndian as Endianness; pub use common::{ read_u32_vint, read_u32_vint_no_advance, serialize_vint_u32, write_u32_vint, VInt, @@ -104,7 +101,6 @@ pub fn u64_to_f64(val: u64) -> f64 { #[cfg(test)] pub(crate) mod test { - //pub use super::serialize::test::fixed_size_test; use super::{f64_to_u64, i64_to_u64, u64_to_f64, u64_to_i64}; use common::{BinarySerializable, FixedSize}; use proptest::prelude::*; diff --git a/src/fastfield/reader.rs b/src/fastfield/reader.rs index 1b093c9f4..8cb7ec089 100644 --- a/src/fastfield/reader.rs +++ b/src/fastfield/reader.rs @@ -73,24 +73,21 @@ pub enum DynamicFastFieldReader { impl DynamicFastFieldReader { /// Returns correct the reader wrapped in the `DynamicFastFieldReader` enum for the data. pub fn open(file: FileSlice) -> crate::Result> { - let bytes = file.read_bytes()?; - let (mut id_bytes, data_bytes) = bytes.split(1); - let id = u8::deserialize(&mut id_bytes)?; + let mut bytes = file.read_bytes()?; + let id = bytes.read_u8(); let reader = match id { BitpackedFastFieldSerializer::>::ID => { DynamicFastFieldReader::Bitpacked(FastFieldReaderCodecWrapper::< Item, BitpackedReader, - >::open_from_bytes(data_bytes)?) + >::open_from_bytes(bytes)?) } LinearInterpolFastFieldSerializer::ID => { DynamicFastFieldReader::LinearInterpol(FastFieldReaderCodecWrapper::< Item, LinearinterpolFastFieldReader, - >::open_from_bytes( - data_bytes - )?) + >::open_from_bytes(bytes)?) } _ => { panic!( diff --git a/src/fastfield/serializer/mod.rs b/src/fastfield/serializer/mod.rs index 8d358326e..8a48199a6 100644 --- a/src/fastfield/serializer/mod.rs +++ b/src/fastfield/serializer/mod.rs @@ -78,7 +78,6 @@ impl CompositeFastFieldSerializer { } estimations.sort_by(|a, b| a.0.partial_cmp(&b.0).unwrap()); let (_ratio, name, id) = estimations[0]; - //estimations.sort_by_key(|el| el.0); id.serialize(field_write)?; match name { BitpackedFastFieldSerializer::>::NAME => {