diff --git a/src/convert/builder.rs b/src/convert/builder.rs index 7c85736..bf27335 100644 --- a/src/convert/builder.rs +++ b/src/convert/builder.rs @@ -255,7 +255,7 @@ impl BestConversions { .map(|n| unit_index.get_unit_id(n)) .collect::, _>>()?; - units.sort_unstable_by(|a, b| { + units.sort_by(|a, b| { let a = &all_units[*a]; let b = &all_units[*b]; a.ratio @@ -365,7 +365,7 @@ fn build_fractions_config( let mut all = None; for cfg in fractions.iter() { - all = cfg.all.map(|c| c.get()); + all = cfg.all.map(|c| c.get()).or(all); } let mut metric = None; @@ -373,8 +373,8 @@ fn build_fractions_config( let mut quantity = HashMap::new(); for cfg in fractions.iter() { - metric = cfg.metric.map(|c| c.get()); - imperial = cfg.imperial.map(|c| c.get()); + metric = cfg.metric.map(|c| c.get()).or(metric); + imperial = cfg.imperial.map(|c| c.get()).or(imperial); for (q, cfg) in &cfg.quantity { quantity.insert(*q, cfg.get()); } diff --git a/src/convert/mod.rs b/src/convert/mod.rs index d628d4a..55d004c 100644 --- a/src/convert/mod.rs +++ b/src/convert/mod.rs @@ -158,6 +158,7 @@ impl Converter { /// /// # Panics /// If the unit is not known. + #[tracing::instrument(level = "trace", skip_all, fields(unit = %unit), ret)] pub(crate) fn fractions_config(&self, unit: &Unit) -> FractionsConfig { let unit_id = self .unit_index @@ -237,7 +238,7 @@ impl Fractions { pub struct FractionsConfig { pub enabled: bool, pub accuracy: f32, - pub max_denominator: u32, + pub max_denominator: u8, pub max_whole: u32, } diff --git a/src/convert/units_file.rs b/src/convert/units_file.rs index 26293d2..cbe274f 100644 --- a/src/convert/units_file.rs +++ b/src/convert/units_file.rs @@ -141,7 +141,8 @@ impl FractionsConfigWrapper { pub struct FractionsConfigHelper { pub enabled: Option, pub accuracy: Option, - pub max_denominator: Option, + #[serde(alias = "max_den")] + pub max_denominator: Option, pub max_whole: Option, } @@ -163,7 +164,7 @@ impl FractionsConfigHelper { max_denominator: self .max_denominator .unwrap_or(d.max_denominator) - .clamp(1, 64), + .clamp(1, 16), max_whole: self.max_whole.unwrap_or(d.max_whole), } } diff --git a/src/quantity.rs b/src/quantity.rs index f24d751..654010a 100644 --- a/src/quantity.rs +++ b/src/quantity.rs @@ -1,13 +1,9 @@ //! Quantity model -use std::{ - collections::{BTreeMap, HashMap, VecDeque}, - fmt::Display, - sync::{Arc, Mutex}, -}; +use std::{collections::HashMap, fmt::Display, sync::Arc}; use enum_map::EnumMap; -use once_cell::sync::OnceCell; +use once_cell::sync::{Lazy, OnceCell}; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -750,82 +746,73 @@ where // All the fractions stuff -struct FractionLookupTable { - max_denom: u32, - table: BTreeMap, -} +static TABLE: Lazy = Lazy::new(FractionLookupTable::new); -impl FractionLookupTable { - const FIX_RATIO: f64 = 1000.0; +#[derive(Debug)] +struct FractionLookupTable(Vec<(i16, (u8, u8))>); - #[tracing::instrument(level = "trace", name = "new_fraction_lookup")] - pub fn new(max_denom: u32) -> Self { - let mut table: BTreeMap = BTreeMap::new(); +impl FractionLookupTable { + const FIX_RATIO: f64 = 1e4; + const DENOMS: &'static [u8] = &[2, 3, 4, 8, 10, 16]; - let denoms = [2, 3, 4, 5, 8, 10, 16, 32, 64]; + pub fn new() -> Self { + debug_assert!(!Self::DENOMS.is_empty()); + debug_assert!(Self::DENOMS.windows(2).all(|w| w[0] < w[1])); + let mut table = Vec::new(); - for den in denoms.into_iter().take_while(|&den| den <= max_denom) { + for &den in Self::DENOMS { for num in 1..den { // not include 1 let val = num as f64 / den as f64; // convert to fixed decimal - let fixed = (val * Self::FIX_RATIO) as i32; + let fixed = (val * Self::FIX_RATIO) as i16; // only insert if not already in // // Because we are iterating from low to high denom, then the value // will only be present with the smallest possible denom. - table.entry(fixed).or_insert((num, den)); + if let Err(pos) = table.binary_search_by_key(&fixed, |&(x, _)| x) { + table.insert(pos, (fixed, (num, den))); + } } } - Self { table, max_denom } - } + table.shrink_to_fit(); - #[tracing::instrument(level = "trace", name = "fraction_table_lookup", skip(self))] - pub fn lookup(&self, value: f64, max_err: f64) -> Option<(u32, u32)> { - let value = (value * Self::FIX_RATIO) as i32; - let max_err = (max_err * Self::FIX_RATIO) as i32; - - self.table - .range(( - std::ops::Bound::Included(value - max_err), - std::ops::Bound::Included(value + max_err), - )) - .min_by_key(|frac_val| (value - frac_val.0).abs()) - .map(|entry| *entry.1) + Self(table) } -} -static FRACTIONS_TABLES: FractionTableCache = FractionTableCache::new(10); + pub fn lookup(&self, val: f64, max_den: u8) -> Option<(u8, u8)> { + let fixed = (val * Self::FIX_RATIO) as i16; + let t = self.0.as_slice(); + let pos = t.binary_search_by_key(&fixed, |&(x, _)| x); -struct FractionTableCache { - size: usize, - cache: Mutex>>, -} - -impl FractionTableCache { - pub const fn new(size: usize) -> Self { - Self { - size, - cache: Mutex::new(VecDeque::new()), + let found = pos.is_ok_and(|i| { + let (x, (_, d)) = t[i]; + x == fixed && d <= max_den + }); + if found { + return Some(t[pos.unwrap()].1); } - } - #[tracing::instrument(level = "trace", name = "fraction_table_cache_get", skip(self))] - pub fn get(&self, max_denom: u32) -> Arc { - let mut cache = self.cache.lock().unwrap(); - // rust borrow checker has some problems here with `find`... idk - if let Some(idx) = cache.iter().position(|t| t.max_denom == max_denom) { - Arc::clone(&cache[idx]) - } else { - if cache.len() == self.size { - cache.pop_front(); + let pos = pos.unwrap_or_else(|i| i); + + let high = t[pos..].iter().find(|(_, (_, d))| *d <= max_den).copied(); + let low = t[..pos].iter().rfind(|(_, (_, d))| *d <= max_den).copied(); + + match (low, high) { + (None, Some((_, f))) | (Some((_, f)), None) => Some(f), + (Some((a_val, a)), Some((b_val, b))) => { + let a_err = (a_val - fixed).abs(); + let b_err = (b_val - fixed).abs(); + if a_err.cmp(&b_err).then(a.1.cmp(&b.1)).is_le() { + Some(a) + } else { + Some(b) + } } - let new = Arc::new(FractionLookupTable::new(max_denom)); - cache.push_back(Arc::clone(&new)); - new + (None, None) => None, } } } @@ -856,7 +843,7 @@ impl Number { /// # Panics /// - If `accuracy > 1` or `accuracy < 0`. /// - If `max_den > 64` - pub fn new_approx(value: f64, accuracy: f32, max_den: u32, max_whole: u32) -> Option { + pub fn new_approx(value: f64, accuracy: f32, max_den: u8, max_whole: u32) -> Option { assert!((0.0..=1.0).contains(&accuracy)); assert!(max_den <= 64); if value <= 0.0 || !value.is_finite() { @@ -887,22 +874,23 @@ impl Number { }); } - let table = FRACTIONS_TABLES.get(max_den); - let (num, den) = table.lookup(decimal, max_err)?; - + let (num, den) = TABLE.lookup(decimal, max_den)?; let approx_value = whole as f64 + num as f64 / den as f64; let err = value - approx_value; + if err.abs() > max_err { + return None; + } Some(Self::Fraction { whole, - num, - den, + num: num as u32, + den: den as u32, err, }) } /// Tries to approximate the number to a fraction if possible and not an /// integer - pub fn try_approx(&mut self, accuracy: f32, max_den: u32, max_whole: u32) -> bool { + pub fn try_approx(&mut self, accuracy: f32, max_den: u8, max_whole: u32) -> bool { match Self::new_approx(self.value(), accuracy, max_den, max_whole) { Some(f) => { *self = f; diff --git a/tests/fractions.rs b/tests/fractions.rs index 12ebcad..dff6b89 100644 --- a/tests/fractions.rs +++ b/tests/fractions.rs @@ -10,7 +10,7 @@ use test_case::test_case; #[test_case(499.999, "lb" => "500 lb")] #[test_case(1.5, "F" => "1.5 °F")] fn imperial(value: f64, unit: &str) -> String { - let converter = Converter::default(); + let converter = Converter::bundled(); let mut q = Quantity::new(Value::from(value), Some(unit.to_string())); let _ = q.convert(System::Imperial, &converter); q.to_string() diff --git a/units.toml b/units.toml index 8d07bb2..6040065 100644 --- a/units.toml +++ b/units.toml @@ -25,7 +25,7 @@ time = false temperature = false [fractions.unit] -tsp = { max_whole = 5, max_denominator = 2 } +tsp = { max_whole = 5, max_denominator = 8 } tbsp = { max_whole = 4, max_denominator = 3 } lb = { max_denominator = 8 }