Skip to content

Commit

Permalink
Refactor term_reducing again; add unit::ops tests
Browse files Browse the repository at this point in the history
  • Loading branch information
turboladen committed Jun 20, 2024
1 parent 52a00d4 commit 39c1f74
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 100 deletions.
2 changes: 1 addition & 1 deletion crates/api/src/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ impl Unit {
#[inline]
#[must_use]
pub fn expression_reduced(&self) -> String {
let reduced = term_reducing::reduce_terms(self.terms.to_vec());
let reduced = term_reducing::reduce_terms(&self.terms);

Self::new(reduced).to_string()
}
Expand Down
192 changes: 110 additions & 82 deletions crates/api/src/unit/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl Div for Unit {
lhs.reserve_exact(other.terms.len());
lhs.extend(other.terms.iter().map(Inv::inv));

Self::new(term_reducing::reduce_terms(lhs))
Self::new(term_reducing::reduce_terms(&lhs))
}
}

Expand All @@ -39,7 +39,7 @@ impl<'a> Div<&'a Self> for Unit {
lhs.reserve_exact(other.terms.len());
lhs.extend(other.terms.iter().map(Inv::inv));

Self::new(term_reducing::reduce_terms(lhs))
Self::new(term_reducing::reduce_terms(&lhs))
}
}

Expand All @@ -56,7 +56,7 @@ impl<'a> Div for &'a Unit {
lhs.reserve_exact(other.terms.len());
lhs.extend(other.terms.iter().map(Inv::inv));

Unit::new(term_reducing::reduce_terms(lhs))
Unit::new(term_reducing::reduce_terms(&lhs))
}
}

Expand All @@ -73,7 +73,7 @@ impl<'a> Div<Unit> for &'a Unit {
lhs.reserve_exact(other.terms.len());
lhs.extend(other.terms.iter().map(Inv::inv));

Unit::new(term_reducing::reduce_terms(lhs))
Unit::new(term_reducing::reduce_terms(&lhs))
}
}

Expand All @@ -96,7 +96,7 @@ impl Mul for Unit {
lhs.append(rhs_unit.terms.to_mut());
}

Self::new(term_reducing::reduce_terms(lhs))
Self::new(term_reducing::reduce_terms(&lhs))
}
}

Expand All @@ -111,7 +111,7 @@ impl<'a> Mul<&'a Self> for Unit {
let mut lhs = self.terms.to_vec();
lhs.extend_from_slice(&other.terms);

Self::new(term_reducing::reduce_terms(lhs))
Self::new(term_reducing::reduce_terms(&lhs))
}
}

Expand All @@ -126,7 +126,7 @@ impl<'a> Mul for &'a Unit {
let mut lhs = self.terms.to_vec();
lhs.extend_from_slice(&other.terms);

Unit::new(term_reducing::reduce_terms(lhs))
Unit::new(term_reducing::reduce_terms(&lhs))
}
}

Expand All @@ -146,7 +146,7 @@ impl<'a> Mul<Unit> for &'a Unit {
lhs.append(rhs_unit.terms.to_mut());
}

Unit::new(term_reducing::reduce_terms(lhs))
Unit::new(term_reducing::reduce_terms(&lhs))
}
}

Expand All @@ -155,7 +155,7 @@ mod tests {
use std::str::FromStr;

use crate::{
testing::const_units::{ACRE, KILOMETER, METER, METER_PER_SECOND},
testing::const_units::{ACRE, KILOMETER, METER, METER_PER_SECOND, PER_SECOND, SECOND},
unit::UNITY,
};

Expand All @@ -168,84 +168,112 @@ mod tests {
mod div {
use super::*;

#[test]
#[allow(clippy::eq_op)]
fn validate_owned_div_owned() {
assert_eq!(METER / METER, UNITY);
let expected = Unit::from_str("m/km").unwrap();
assert_eq!(METER / KILOMETER, expected);
let unit = Unit::from_str("10m").unwrap();
let other = Unit::from_str("20m").unwrap();
let expected = Unit::from_str("10m/20m").unwrap();
assert_eq!(unit / other, expected);

assert_eq!(seed() / seed(), UNITY);
assert_eq!(UNITY / seed(), Unit::from_str("/{seed}").unwrap());
assert_eq!(seed() / ACRE, Unit::from_str("{seed}/[acr_us]").unwrap());
macro_rules! test_div {
($test_name:ident: $lhs:expr, $rhs:expr => $expected:expr) => {
#[test]
fn $test_name() {
// Borrowed / Borrowed
{
let result = &$lhs / &$rhs;
assert_field_eq!(result, &$expected, "Actual: {:#?}", result);
}

// Owned / Borrowed
{
let result = $lhs / &$rhs;
assert_field_eq!(result, &$expected, "Actual: {:#?}", result);
}

// Borrowed / Owned
{
let result = &$lhs / $rhs;
assert_field_eq!(result, &$expected, "Actual: {:#?}", result);
}

// Owned / Owned
{
let result = $lhs / $rhs;
assert_field_eq!(result, &$expected, "Actual: {:#?}", result);
}
}
};
}

#[test]
fn validate_owned_div_borrowed() {
assert_eq!(METER / &METER, UNITY);
let expected = Unit::from_str("m/km").unwrap();
assert_eq!(METER / &KILOMETER, expected);
let unit = Unit::from_str("10m").unwrap();
let other = Unit::from_str("20m").unwrap();
let expected = Unit::from_str("10m/20m").unwrap();
assert_eq!(unit / &other, expected);

assert_eq!(seed() / &seed(), UNITY);
assert_eq!(UNITY / &seed(), Unit::from_str("/{seed}").unwrap());
assert_eq!(seed() / &ACRE, Unit::from_str("{seed}/[acr_us]").unwrap());
}

#[test]
fn validate_borrowed_div_owned() {
assert_eq!(&METER / METER, UNITY);
let expected = Unit::from_str("m/km").unwrap();
assert_eq!(&METER / KILOMETER, expected);
let unit = Unit::from_str("10m").unwrap();
let other = Unit::from_str("20m").unwrap();
let expected = Unit::from_str("10m/20m").unwrap();
assert_eq!(&unit / other, expected);

assert_eq!(&seed() / seed(), UNITY);
assert_eq!(&UNITY / seed(), Unit::from_str("/{seed}").unwrap());
assert_eq!(&seed() / ACRE, Unit::from_str("{seed}/[acr_us]").unwrap());
}

#[test]
fn validate_borrowed_div_borrowed() {
assert_eq!(&METER / &METER, UNITY);
let expected = Unit::from_str("m/km").unwrap();
assert_eq!(&METER / &KILOMETER, expected);
let unit = Unit::from_str("10m").unwrap();
let other = Unit::from_str("20m").unwrap();
let expected = Unit::from_str("10m/20m").unwrap();
assert_eq!(&unit / &other, expected);

assert_eq!(&seed() / &seed(), UNITY);
assert_eq!(&UNITY / &seed(), Unit::from_str("/{seed}").unwrap());
assert_eq!(&seed() / &ACRE, Unit::from_str("{seed}/[acr_us]").unwrap());
}
test_div!(test_atom_div_same_atom:
METER, METER => UNITY);
test_div!(test_atom_div_different_atom:
METER, SECOND => METER_PER_SECOND);
test_div!(test_atom_div_prefix_same_atom:
METER, KILOMETER => parse_unit!("m/km"));
test_div!(test_factor_atom_div_factor_same_atom:
parse_unit!("10m"), parse_unit!("20m") => parse_unit!("10m/20m"));
test_div!(test_nondim_div_same_nondim:
seed(), seed() => parse_unit!("{seed}/{seed}"));
test_div!(test_unity_div_same_nondim:
UNITY, seed() => Unit::new(vec![crate::term::UNITY, term!(factor: 1, exponent: -1, annotation: "seed")]));
test_div!(test_nondim_div_atom:
seed(), ACRE => parse_unit!("{seed}/[acr_us]"));
test_div!(test_atom_div_per_atom:
METER, PER_SECOND => parse_unit!("m.s"));
test_div!(test_atom_div_per_atom_per_atom:
METER, METER_PER_SECOND => parse_unit!("1.s"));
test_div!(test_annotatable_div_different_annotatable:
parse_unit!("42m{foo}"), parse_unit!("42m{bar}") => parse_unit!("42m{foo}/42m{bar}"));
}

#[test]
fn validate_mul() {
let expected = Unit::from_str("m.km").unwrap();
assert_eq!(METER * KILOMETER, expected);

let unit = Unit::from_str("10m").unwrap();
let other = Unit::from_str("20m").unwrap();
let expected = Unit::from_str("10m.20m").unwrap();
assert_eq!(unit * other, expected);

let per_seed = Unit::from_str("/{seed}").unwrap();
assert_eq!(seed() * &per_seed, UNITY);
mod mul {
use super::*;

let seed_per_acre = Unit::from_str("{seed}/[acr_us]").unwrap();
assert_eq!(seed_per_acre * ACRE, seed());
macro_rules! test_mul {
($test_name:ident: $lhs:expr, $rhs:expr => $expected:expr) => {
#[test]
fn $test_name() {
// Borrowed / Borrowed
{
let result = &$lhs * &$rhs;
assert_field_eq!(result, &$expected, "Actual: {:#?}", result);
}

// Owned / Borrowed
{
let result = $lhs * &$rhs;
assert_field_eq!(result, &$expected, "Actual: {:#?}", result);
}

// Borrowed / Owned
{
let result = &$lhs * $rhs;
assert_field_eq!(result, &$expected, "Actual: {:#?}", result);
}

// Owned / Owned
{
let result = $lhs * $rhs;
assert_field_eq!(result, &$expected, "Actual: {:#?}", result);
}
}
};
}

assert_eq!(METER_PER_SECOND * Unit::from_str("s/m").unwrap(), UNITY);
test_mul!(test_atom_mul_same_atom:
METER, METER => parse_unit!["m2"]);
test_mul!(test_atom_mul_different_atom:
METER, SECOND => parse_unit!["m.s"]);
test_mul!(test_atom_mul_prefix_same_atom:
METER, KILOMETER => parse_unit!["m.km"]);
test_mul!(test_factor_atom_mul_factor_same_atom:
parse_unit!("10m"), parse_unit!("20m") => parse_unit!["10m.20m"]);
test_mul!(test_nondim_mul_same_nondim:
seed(), seed() => unit!(term!(factor: 1, exponent: 2, annotation: "seed")));
test_mul!(test_unity_mul_same_nondim:
UNITY, seed() => parse_unit!["1.{seed}"]);
test_mul!(test_nondim_mul_atom:
seed(), ACRE => parse_unit!["{seed}.[acr_us]"]);
test_mul!(test_atom_mul_per_atom:
METER, PER_SECOND => METER_PER_SECOND);
test_mul!(test_atom_mul_per_atom_per_atom:
METER, METER_PER_SECOND => parse_unit!["m2/s"]);
test_mul!(test_annotatable_mul_different_annotatable:
parse_unit!("42m{foo}"), parse_unit!("42m-1{bar}") => parse_unit!("42m{foo}/42m{bar}"));
}
}
42 changes: 25 additions & 17 deletions crates/api/src/unit/term_reducing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,37 +25,34 @@ use crate::Term;
/// result of this call (if we passed a reference/slice instead, we'd have to clone the originals,
/// and this is most likely to happen).
///
pub(super) fn reduce_terms(terms: Vec<Term>) -> Cow<'static, [Term]> {
pub(super) fn reduce_terms(terms: &[Term]) -> Cow<'static, [Term]> {
let mut ids_to_remove = vec![];
let mut output = terms.clone();
let mut output = vec![];

for (i, lhs) in output.iter_mut().enumerate() {
for (i, lhs) in terms.iter().enumerate() {
if ids_to_remove.contains(&i) {
continue;
}

let mut offset = i + 1;
let mut new_term = lhs.clone();

'inner: for (j, rhs) in terms[offset..].iter().enumerate() {
if ids_to_remove.contains(&(offset + j)) {
continue 'inner;
}

if composably_eq(lhs, rhs) {
let new_exponent = lhs.effective_exponent() + rhs.effective_exponent();
let new_exponent = new_term.effective_exponent() + rhs.effective_exponent();

let _ = lhs.set_exponent(new_exponent);
let _ = new_term.set_exponent(new_exponent);

ids_to_remove.push(offset);
}
offset += 1;
}
}

ids_to_remove.sort_unstable();
ids_to_remove.reverse();

for id in ids_to_remove {
let _ = output.remove(id);
output.push(new_term);
}

// If everything is reduced away, the effective Unit should be "1".
Expand All @@ -75,11 +72,22 @@ fn composably_eq(lhs: &Term, rhs: &Term) -> bool {

#[cfg(test)]
mod tests {
use crate::{
term::{
variants::{
FactorPrefixAtomAnnotation, FactorPrefixAtomExponentAnnotation, PrefixAtom,
PrefixAtomExponent,
},
UNITY,
},
Annotation, Atom, Prefix,
};

use super::*;

#[test]
fn empty_terms_test() {
assert_eq!(reduce_terms(vec![]), vec![UNITY]);
assert_eq!(reduce_terms(&[]), vec![UNITY]);
}

#[test]
Expand All @@ -92,7 +100,7 @@ mod tests {
3,
Annotation::from("foo"),
));
assert_eq!(reduce_terms(vec![term.clone()]), vec![term]);
assert_eq!(reduce_terms(&[term.clone()]), vec![term]);
}

// NOTE: The two terms only differ in their `annotation`s here. (and `exponent`, but that field
Expand All @@ -117,7 +125,7 @@ mod tests {
Annotation::from("bar"),
));
assert_eq!(
reduce_terms(vec![term1.clone(), term2.clone()]),
reduce_terms(&[term1.clone(), term2.clone()]),
vec![term1, term2]
);
}
Expand All @@ -142,7 +150,7 @@ mod tests {
));

assert_eq!(
reduce_terms(vec![term1, term2]),
reduce_terms(&[term1, term2]),
vec![Term::FactorPrefixAtomAnnotation(
FactorPrefixAtomAnnotation::new(
42,
Expand All @@ -162,7 +170,7 @@ mod tests {
let term3 = Term::PrefixAtom(PrefixAtom::new(Prefix::Kilo, Atom::Meter));

assert_eq!(
reduce_terms(vec![term1, term2, term3]),
reduce_terms(&[term1, term2, term3]),
vec![Term::PrefixAtomExponent(PrefixAtomExponent::new(
Prefix::Kilo,
Atom::Meter,
Expand All @@ -179,7 +187,7 @@ mod tests {
let term3 = Term::PrefixAtomExponent(PrefixAtomExponent::new(Prefix::Kilo, Atom::Gram, 2));

assert_eq!(
reduce_terms(vec![term1, term2, term3]),
reduce_terms(&[term1, term2, term3]),
vec![
Term::PrefixAtom(PrefixAtom::new(Prefix::Kilo, Atom::Meter,)),
Term::PrefixAtomExponent(PrefixAtomExponent::new(Prefix::Kilo, Atom::Gram, 2))
Expand Down

0 comments on commit 39c1f74

Please sign in to comment.