Make unit simplification more consistent

Units like `%` and `million` are now simplified unless you
explicitly convert your result to one of those units.

For example:
```
> 5%
0.05
> 46 million
46000000
> 0.5 to %
50%
> 34820000 to million
34.82 million
```

Previously `5%` used to return `5%` and `34820000 to million`
used to return an error.

CHANGELOG.md

@@ -1,5 +1,23 @@
 ## Changelog
 
+### Unreleased
+
+* Change unit simplification and unit aliasing to be simpler and more
+    consistent. Units like `%` and `million` are now simplified unless you
+    explicitly convert your result to one of those units.
+
+    For example:
+    ```
+    > 5%
+    0.05
+    > 46 million
+    46000000
+    > 0.5 to %
+    50%
+    > 34820000 to million
+    34.82 million
+    ```
+
 ### v1.3.3 (2023-12-08)
 
 * Add `pkgx` package (by [@michaelessiet](https://github.com/michaelessiet))

core/src/ast.rs

@@ -614,6 +614,7 @@ pub(crate) fn resolve_identifier<I: Interrupt>(
 		"real" | "re" | "Re" => Value::BuiltInFunction(BuiltInFunction::Real),
 		"imag" | "im" | "Im" => Value::BuiltInFunction(BuiltInFunction::Imag),
 		"conjugate" => Value::BuiltInFunction(BuiltInFunction::Conjugate),
+		"unitless" => Value::Num(Box::new(Number::from(1))),
 		"arg" => Value::BuiltInFunction(BuiltInFunction::Arg),
 		"abs" => Value::BuiltInFunction(BuiltInFunction::Abs),
 		"sin" => Value::BuiltInFunction(BuiltInFunction::Sin),

core/src/num/unit.rs

@@ -72,14 +72,21 @@ impl Value {
 	pub(crate) fn create_unit_value_from_value<I: Interrupt>(
 		value: &Self,
 		prefix: Cow<'static, str>,
+		alias: bool,
 		singular_name: Cow<'static, str>,
 		plural_name: Cow<'static, str>,
 		int: &I,
 	) -> Result<Self, FendError> {
 		let (hashmap, scale) = value.unit.to_hashmap_and_scale(int)?;
 		let scale = scale.mul(&Exact::new(value.value.one_point_ref()?.clone(), true), int)?;
-		let resulting_unit =
-			NamedUnit::new(prefix, singular_name, plural_name, hashmap, scale.value);
+		let resulting_unit = NamedUnit::new(
+			prefix,
+			singular_name,
+			plural_name,
+			alias,
+			hashmap,
+			scale.value,
+		);
 		let mut result = Self::new(1, vec![UnitExponent::new(resulting_unit, 1)]);
 		result.exact = result.exact && value.exact && scale.exact;
 		Ok(result)
@@ -92,7 +99,14 @@ impl Value {
 		let base_unit = BaseUnit::new(singular_name.clone());
 		let mut hashmap = HashMap::new();
 		hashmap.insert(base_unit, 1.into());
-		let unit = NamedUnit::new(Cow::Borrowed(""), singular_name, plural_name, hashmap, 1);
+		let unit = NamedUnit::new(
+			Cow::Borrowed(""),
+			singular_name,
+			plural_name,
+			false,
+			hashmap,
+			1,
+		);
 		Self::new(1, vec![UnitExponent::new(unit, 1)])
 	}
 
@@ -335,7 +349,7 @@ impl Value {
 		}
 	}
 
-	fn is_unitless<I: Interrupt>(&self, int: &I) -> Result<bool, FendError> {
+	pub(crate) fn is_unitless<I: Interrupt>(&self, int: &I) -> Result<bool, FendError> {
 		// todo this is broken for unitless components
 		if self.unit.components.is_empty() {
 			return Ok(true);
@@ -722,57 +736,25 @@ impl Value {
 		let mut res_exact = self.exact;
 		let mut res_value = self.value;
 
-		/*
-		 * In fend, percentages are units.
-		 * Without any special handling, multiplication would
-		 * unconditionally combine them.
-		 *
-		 * This would (and did) lead to unexpected results,
-		 * that fell into two broad categories:
-		 * 1. mixing percentages with units: `80 kg * 5% = 400 kg %`.
-		 * 2. percenteges squared: "5% * 5% = 25%^2"
-		 *
-		 * In practice, percentages are usually used as scalar multipliers,
-		 * not as "units" in the traditional sense.
-		 *
-		 * To avoid this, we have the following rules
-		 * for simplifying percentages.
-		 *
-		 * 1. If there are any other units (kg, lbs, etc..),
-		 *    then all of the percentages are removed (fixing first problem)
-		 * 2. No more than one "percentage unit" is permitted.
-		 *
-		 * This (mostly) fixes issue #164
-		 *
-		 * There is still some ambiguity here even after
-		 * going through these rules (although this fixes most of it).
-		 * See discussion on the "5_percent_times_100" test for more details.
-		 */
-		let mut have_percentage_unit = false;
-		let has_nonpercentage_components =
-			self.unit.components.iter().any(|u| !u.is_percentage_unit());
-		// combine identical or compatible units by summing their exponents
-		// and potentially adjusting the value
+		// remove alias units and combine identical or compatible units
+		// by summing their exponents and potentially adjusting the value
 		'outer: for comp in self.unit.components {
-			if comp.is_percentage_unit() {
-				if have_percentage_unit || has_nonpercentage_components {
-					let adjusted_res = Exact {
-						value: res_value,
-						exact: res_exact,
-					}
-					.mul(
-						&Exact {
-							value: comp.unit.scale.clone().into(),
-							exact: true,
-						},
-						int,
-					)?;
-					res_value = adjusted_res.value;
-					res_exact = adjusted_res.exact;
-					continue 'outer;
+			if comp.is_alias() {
+				// remove alias units
+				let adjusted_res = Exact {
+					value: res_value,
+					exact: res_exact,
 				}
-				// already encountered one (if we see another one, strip it)
-				have_percentage_unit = true;
+				.mul(
+					&Exact {
+						value: comp.unit.scale.clone().into(),
+						exact: true,
+					},
+					int,
+				)?;
+				res_value = adjusted_res.value;
+				res_exact = adjusted_res.exact;
+				continue;
 			}
 			for res_comp in &mut res_components {
 				if comp.unit.has_no_base_units() && comp.unit != res_comp.unit {
@@ -1217,7 +1199,7 @@ mod tests {
 		let base_kg = BaseUnit::new("kilogram".into());
 		let mut hashmap = HashMap::new();
 		hashmap.insert(base_kg, 1.into());
-		let kg = NamedUnit::new("k".into(), "g".into(), "g".into(), hashmap, 1);
+		let kg = NamedUnit::new("k".into(), "g".into(), "g".into(), false, hashmap, 1);
 		let one_kg = Value::new(1, vec![UnitExponent::new(kg.clone(), 1)]);
 		let two_kg = Value::new(2, vec![UnitExponent::new(kg, 1)]);
 		let sum = one_kg.add(two_kg, &Never).unwrap();
@@ -1230,11 +1212,19 @@ mod tests {
 		let base_kg = BaseUnit::new("kilogram".into());
 		let mut hashmap = HashMap::new();
 		hashmap.insert(base_kg, 1.into());
-		let kg = NamedUnit::new("k".into(), "g".into(), "g".into(), hashmap.clone(), 1);
+		let kg = NamedUnit::new(
+			"k".into(),
+			"g".into(),
+			"g".into(),
+			false,
+			hashmap.clone(),
+			1,
+		);
 		let g = NamedUnit::new(
 			Cow::Borrowed(""),
 			Cow::Borrowed("g"),
 			Cow::Borrowed("g"),
+			false,
 			hashmap,
 			Exact::new(Complex::from(1), true)
 				.div(Exact::new(1000.into(), true), int)

core/src/num/unit/named_unit.rs

@@ -4,7 +4,10 @@ use super::base_unit::BaseUnit;
 use crate::{
 	error::FendError,
 	num::complex::Complex,
-	serialize::{deserialize_string, deserialize_usize, serialize_string, serialize_usize},
+	serialize::{
+		deserialize_bool, deserialize_string, deserialize_usize, serialize_bool, serialize_string,
+		serialize_usize,
+	},
 };
 
 /// A named unit, like kilogram, megabyte or percent.
@@ -13,6 +16,7 @@ pub(crate) struct NamedUnit {
 	prefix: Cow<'static, str>,
 	singular_name: Cow<'static, str>,
 	plural_name: Cow<'static, str>,
+	alias: bool,
 	pub(super) base_units: HashMap<BaseUnit, Complex>,
 	pub(super) scale: Complex,
 }
@@ -22,13 +26,15 @@ impl NamedUnit {
 		prefix: Cow<'static, str>,
 		singular_name: Cow<'static, str>,
 		plural_name: Cow<'static, str>,
+		alias: bool,
 		base_units: HashMap<BaseUnit, Complex>,
 		scale: impl Into<Complex>,
 	) -> Self {
 		Self {
 			prefix,
 			singular_name,
 			plural_name,
+			alias,
 			base_units,
 			scale: scale.into(),
 		}
@@ -38,6 +44,7 @@ impl NamedUnit {
 		serialize_string(self.prefix.as_ref(), write)?;
 		serialize_string(self.singular_name.as_ref(), write)?;
 		serialize_string(self.plural_name.as_ref(), write)?;
+		serialize_bool(self.alias, write)?;
 
 		serialize_usize(self.base_units.len(), write)?;
 		for (a, b) in &self.base_units {
@@ -53,6 +60,7 @@ impl NamedUnit {
 		let prefix = deserialize_string(read)?;
 		let singular_name = deserialize_string(read)?;
 		let plural_name = deserialize_string(read)?;
+		let alias = deserialize_bool(read)?;
 
 		let len = deserialize_usize(read)?;
 		let mut hashmap = HashMap::with_capacity(len);
@@ -65,6 +73,7 @@ impl NamedUnit {
 			prefix: Cow::Owned(prefix),
 			singular_name: Cow::Owned(singular_name),
 			plural_name: Cow::Owned(plural_name),
+			alias,
 			base_units: hashmap,
 			scale: Complex::deserialize(read)?,
 		})
@@ -75,6 +84,7 @@ impl NamedUnit {
 			prefix: "".into(),
 			singular_name: base_unit.name().to_string().into(),
 			plural_name: base_unit.name().to_string().into(),
+			alias: false,
 			base_units: {
 				let mut base_units = HashMap::new();
 				base_units.insert(base_unit, 1.into());
@@ -99,6 +109,10 @@ impl NamedUnit {
 		self.base_units.is_empty()
 	}
 
+	pub(crate) fn is_alias(&self) -> bool {
+		self.alias && self.has_no_base_units()
+	}
+
 	/// Returns whether or not this unit should be printed with a
 	/// space (between the number and the unit). This should be true for most
 	/// units like kg or m, but not for % or °

core/src/num/unit/unit_exponent.rs

@@ -39,9 +39,8 @@ impl UnitExponent {
 		})
 	}
 
-	pub(crate) fn is_percentage_unit(&self) -> bool {
-		let (prefix, name) = self.unit.prefix_and_name(false);
-		prefix.is_empty() && ["%", "percent"].contains(&name)
+	pub(crate) fn is_alias(&self) -> bool {
+		self.unit.is_alias()
 	}
 
 	pub(crate) fn add_to_hashmap<I: Interrupt>(

core/src/units.rs

@@ -24,6 +24,7 @@ pub(crate) struct UnitDef {
 	singular: Cow<'static, str>,
 	plural: Cow<'static, str>,
 	prefix_rule: PrefixRule,
+	alias: bool,
 	value: Value,
 }
 
@@ -51,6 +52,7 @@ fn expr_unit<I: Interrupt>(
 		let value = Number::create_unit_value_from_value(
 			&value,
 			Cow::Borrowed(""),
+			false,
 			singular.clone(),
 			plural.clone(),
 			int,
@@ -59,6 +61,7 @@ fn expr_unit<I: Interrupt>(
 			singular,
 			plural,
 			prefix_rule: PrefixRule::LongPrefixAllowed,
+			alias: false,
 			value: Value::Num(Box::new(value)),
 		});
 	}
@@ -88,16 +91,20 @@ fn expr_unit<I: Interrupt>(
 			prefix_rule: rule,
 			singular,
 			plural,
+			alias: false,
 		});
 	}
 	let (alias, definition) = definition
 		.strip_prefix('=')
 		.map_or((false, definition), |remaining| (true, remaining));
 	let mut num = evaluate_to_value(definition, None, attrs, context, int)?.expect_num()?;
-	if !alias && rule != PrefixRule::LongPrefix {
+	if rule == PrefixRule::LongPrefix || (alias && !num.is_unitless(int)?) {
+		// keep full unit
+	} else {
 		num = Number::create_unit_value_from_value(
 			&num,
 			Cow::Borrowed(""),
+			alias,
 			singular.clone(),
 			plural.clone(),
 			int,
@@ -108,6 +115,7 @@ fn expr_unit<I: Interrupt>(
 		prefix_rule: rule,
 		singular,
 		plural,
+		alias,
 	})
 }
 
@@ -118,8 +126,9 @@ fn construct_prefixed_unit<I: Interrupt>(
 ) -> Result<Value, FendError> {
 	let product = a.value.expect_num()?.mul(b.value.expect_num()?, int)?;
 	assert_eq!(a.singular, a.plural);
-	let unit =
-		Number::create_unit_value_from_value(&product, a.singular, b.singular, b.plural, int)?;
+	let unit = Number::create_unit_value_from_value(
+		&product, a.singular, b.alias, b.singular, b.plural, int,
+	)?;
 	Ok(Value::Num(Box::new(unit)))
 }
 

core/src/units/builtin.rs

@@ -432,9 +432,9 @@ const TIME_UNITS: &[UnitTuple] = &[
 ];
 
 const RATIOS: &[UnitTuple] = &[
-	("\u{2030}", "", "0.001", ""), // per mille
-	("percent", "", "0.01", ""),
-	("%", "", "percent", ""),
+	("\u{2030}", "", "=0.001", ""), // per mille
+	("percent", "", "=0.01", ""),
+	("%", "", "=percent", ""),
 	("bel", "bels", "0.5 * ln(10) neper", ""),
 	("decibel", "decibels", "1/10 bel", ""),
 	("dB", "", "decibel", ""),