Update to quickcheck 1.0.

Change required the MSRV update as well as refactoring `from_base`, `to_base`, and multiple tests. The `from_base` and `to_base` changes provide better floating point precision while still maintaing zero-cost guarantees. The test changes better handle floating point precision issues as well as the wider range of values generated by `quickcheck` 1.0's `Arbitrary` implementation.
iliekturtles · Aug 18, 2021 · cad039c · cad039c
1 parent db38be2
commit cad039c
Show file tree

Hide file tree

Showing 8 changed files with 256 additions and 156 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -41,7 +41,7 @@ typenum = "1.13"
 
 [dev-dependencies]
 approx = "0.5"
-quickcheck = "0.9.2"
+quickcheck = "1.0"
 serde_json = "1.0"
 static_assertions = "1.1"
 

diff --git a/src/lib.rs b/src/lib.rs
@@ -438,8 +438,10 @@ pub trait Conversion<V> {
 /// * `V`: Underlying storage type trait is implemented for.
 ///
 /// [factor]: https://jcgm.bipm.org/vim/en/1.24.html
+#[allow(unused_qualifications)] // lib:cmp::PartialOrder false positive.
 pub trait ConversionFactor<V>:
-    lib::ops::Add<Self, Output = Self>
+    lib::cmp::PartialOrd
+    + lib::ops::Add<Self, Output = Self>
     + lib::ops::Sub<Self, Output = Self>
     + lib::ops::Mul<Self, Output = Self>
     + lib::ops::Div<Self, Output = Self>

diff --git a/src/si/ratio.rs b/src/si/ratio.rs
@@ -161,33 +161,29 @@ mod tests {
             use crate::si::quantities::*;
             use crate::tests::Test;
 
-            fn test_nan_or_eq(yl: V, yr: V) -> bool {
-                (yl.is_nan() && yr.is_nan()) || Test::eq(&yl, &yr)
-            }
-
             quickcheck! {
                 fn acos(x: V) -> bool {
-                    test_nan_or_eq(x.acos(), Ratio::from(x).acos().get::<a::radian>())
+                    Test::eq(&x.acos(), &Ratio::from(x).acos().get::<a::radian>())
                 }
 
                 fn acosh(x: V) -> bool {
-                    test_nan_or_eq(x.acosh(), Ratio::from(x).acosh().get::<a::radian>())
+                    Test::eq(&x.acosh(), &Ratio::from(x).acosh().get::<a::radian>())
                 }
 
                 fn asin(x: V) -> bool {
-                    test_nan_or_eq(x.asin(), Ratio::from(x).asin().get::<a::radian>())
+                    Test::eq(&x.asin(), &Ratio::from(x).asin().get::<a::radian>())
                 }
 
                 fn asinh(x: V) -> bool {
-                    test_nan_or_eq(x.asinh(), Ratio::from(x).asinh().get::<a::radian>())
+                    Test::eq(&x.asinh(), &Ratio::from(x).asinh().get::<a::radian>())
                 }
 
                 fn atan(x: V) -> bool {
-                    test_nan_or_eq(x.atan(), Ratio::from(x).atan().get::<a::radian>())
+                    Test::eq(&x.atan(), &Ratio::from(x).atan().get::<a::radian>())
                 }
 
                 fn atanh(x: V) -> bool {
-                    test_nan_or_eq(x.atanh(), Ratio::from(x).atanh().get::<a::radian>())
+                    Test::eq(&x.atanh(), &Ratio::from(x).atanh().get::<a::radian>())
                 }
             }
         }

diff --git a/src/si/time.rs b/src/si/time.rs
@@ -124,7 +124,7 @@ where
 
     fn try_from(duration: Duration) -> Result<Self, Self::Error> {
         let secs = V::from_u64(duration.as_secs());
-        let nanos = V::from_u32(duration.subsec_micros());
+        let nanos = V::from_u32(duration.subsec_nanos());
 
         match (secs, nanos) {
             (Some(secs), Some(nanos)) => {
@@ -140,73 +140,54 @@ mod tests {
     storage_types! {
         types: PrimInt, BigInt, BigUint, Float;
 
-        use crate::lib::convert::{TryFrom, TryInto};
+        use crate::ConversionFactor;
+        use crate::lib::convert::TryFrom;
         use crate::lib::time::Duration;
-        use crate::num::{FromPrimitive, ToPrimitive, Zero};
+        use crate::num::{FromPrimitive, ToPrimitive, One, Zero};
         use crate::si::quantities::*;
-        use crate::si::time::{TryFromError, nanosecond};
+        use crate::si::time::{TryFromError, second, nanosecond};
         use crate::tests::*;
         use quickcheck::TestResult;
 
         quickcheck! {
-            fn duration_try_from(v: A<V>) -> TestResult {
-                test_try_from(Duration::try_from(Time::new::<nanosecond>((*v).clone())), v)
-            }
-
-            fn time_try_into(v: A<V>) -> TestResult {
-                test_try_from(Time::new::<nanosecond>((*v).clone()).try_into(), v)
-            }
-
-            fn time_try_from(v: A<V>) -> TestResult {
-                match v.to_u64() {
-                    Some(u) => test_try_into(Time::try_from(Duration::from_nanos(u)), v),
-                    None => TestResult::discard(),
+            fn duration_try_from(v: A<V>) -> bool {
+                let ns: V = <nanosecond as crate::Conversion<V>>::coefficient().value();
+                let t = Time::new::<second>((*v).clone());
+                let d = Duration::try_from(t);
+                let r = (*v).clone() % V::one();
+                let s = ((*v).clone() - r.clone()).to_u64();
+                let n = (r * (V::one() / &ns)).to_u32();
+
+                match (d, s, n) {
+                    (Ok(d), Some(s), Some(n)) => d.as_secs() == s && d.subsec_nanos() == n,
+                    (Err(TryFromError::NegativeDuration), _, _) if *v < V::zero() => true,
+                    (Err(TryFromError::Overflow), None, _) => true,
+                    (Err(TryFromError::Overflow), _, None) => true,
+                    _ => false,
                 }
             }
 
-            fn duration_try_into(v: A<V>) -> TestResult {
-                match v.to_u64() {
-                    Some(u) => test_try_into(Duration::from_nanos(u).try_into(), v),
-                    None => TestResult::discard(),
+            fn time_try_from(v: A<V>) -> TestResult {
+                if *v < V::zero()  {
+                    return TestResult::discard();
                 }
-            }
-        }
 
-        fn test_try_from(t: Result<Duration, TryFromError>, v: A<V>) -> TestResult {
-            if *v < V::zero() {
-                return TestResult::discard();
+                let ns: V = <nanosecond as crate::Conversion<V>>::coefficient().value();
+                let r = (*v).clone() % V::one();
+                let s = ((*v).clone() - r.clone()).to_u64();
+                let n = (r * (V::one() / &ns)).to_u32();
+
+                return match (s, n) {
+                        (Some(s), Some(n)) => TestResult::from_bool(
+                            match (Time::try_from(Duration::new(s, n)), V::from_u64(s), V::from_u32(n)) {
+                                (Ok(t), Some(s), Some(n)) => t == Time::new::<second>(s) + Time::new::<nanosecond>(n),
+                                (Err(TryFromError::Overflow), None, _) => true,
+                                (Err(TryFromError::Overflow), _, None) => true,
+                                _ => false,
+                            }),
+                        _ => TestResult::discard(),
+                    }
             }
-
-            let ok = match (t, v.to_u64()) {
-                (Ok(t), Some(u)) => {
-                    let d = Duration::from_nanos(u);
-                    let r = if d > t { d - t } else { t - d };
-
-                    Duration::from_nanos(1) >= r
-                },
-                (Err(_), None) => true,
-                _ => false,
-            };
-
-            TestResult::from_bool(ok)
-        }
-
-        fn test_try_into(t: Result<Time<V>, TryFromError>, v: A<V>) -> TestResult {
-            if *v < V::zero() {
-                return TestResult::discard();
-            }
-
-            let ok = match t {
-                Ok(t) => {
-                    let b = Time::new::<nanosecond>((*v).clone());
-                    let d = if t > b { t - b } else { b - t };
-
-                    d <= Time::new::<nanosecond>(V::from_u8(100).unwrap())
-                },
-                _ => false,
-            };
-
-            TestResult::from_bool(ok)
         }
     }
 }
diff --git a/src/system.rs b/src/system.rs
@@ -303,16 +303,23 @@ macro_rules! system {
         where
             D: Dimension + ?Sized,
             U: Units<V> + ?Sized,
-            V: $crate::Conversion<V> + $crate::lib::ops::Mul<V, Output = V>,
+            V: $crate::Conversion<V>,
             N: $crate::Conversion<V, T = V::T>,
         {
             use $crate::typenum::Integer;
-            use $crate::Conversion;
-            use $crate::ConversionFactor;
+            use $crate::{Conversion, ConversionFactor};
 
-            (v.conversion() $(* U::$name::coefficient().powi(D::$symbol::to_i32()))+
-                    / N::coefficient() - N::constant($crate::ConstantOp::Sub))
-                .value()
+            let v = v.conversion();
+            let n_coef = N::coefficient();
+            let f = V::coefficient() $(* U::$name::coefficient().powi(D::$symbol::to_i32()))+;
+            let n_cons = N::constant($crate::ConstantOp::Sub);
+
+            if n_coef < f {
+                (v * (f / n_coef) - n_cons).value()
+            }
+            else {
+                (v / (n_coef / f) - n_cons).value()
+            }
         }
 
         /// Convert a value from the given unit to base units.
@@ -327,16 +334,23 @@ macro_rules! system {
         where
             D: Dimension + ?Sized,
             U: Units<V> + ?Sized,
-            V: $crate::Conversion<V> + $crate::lib::ops::Mul<V, Output = V>,
+            V: $crate::Conversion<V>,
             N: $crate::Conversion<V, T = V::T>,
         {
             use $crate::typenum::Integer;
-            use $crate::Conversion;
-            use $crate::ConversionFactor;
+            use $crate::{Conversion, ConversionFactor};
 
-            ((v.conversion() + N::constant($crate::ConstantOp::Add)) * N::coefficient()
-                    / (V::coefficient() $(* U::$name::coefficient().powi(D::$symbol::to_i32()))+))
-                .value()
+            let v = v.conversion();
+            let n_coef = N::coefficient();
+            let f = V::coefficient() $(* U::$name::coefficient().powi(D::$symbol::to_i32()))+;
+            let n_cons = N::constant($crate::ConstantOp::Add);
+
+            if n_coef >= f {
+                ((v + n_cons) * (n_coef / f)).value()
+            }
+            else {
+                (((v + n_cons) * n_coef) / f).value()
+            }
         }
 
         autoconvert_test! {
@@ -357,8 +371,7 @@ macro_rules! system {
             V: $crate::Conversion<V> + $crate::lib::ops::Mul<V, Output = V>,
         {
             use $crate::typenum::Integer;
-            use $crate::Conversion;
-            use $crate::ConversionFactor;
+            use $crate::{Conversion, ConversionFactor};
 
             (v.conversion() $(* Ur::$name::coefficient().powi(D::$symbol::to_i32())
                     / Ul::$name::coefficient().powi(D::$symbol::to_i32()))+)

diff --git a/src/tests/mod.rs b/src/tests/mod.rs
@@ -108,12 +108,37 @@ mod test_trait {
         const ULPS: u32 = 3;
 
         impl super::super::Test for V {
+            /// Assert that `lhs` and `rhs` are exactly equal.
+            fn assert_eq(lhs: &Self, rhs: &Self) {
+                match (lhs.is_nan(), rhs.is_nan()) {
+                    (true, true) => {}
+                    _ => { assert_eq!(lhs, rhs); }
+                }
+            }
+
+            /// Assert that `lhs` and `rhs` are approximately equal for floating point types or
+            /// exactly equal for other types.
             fn assert_approx_eq(lhs: &Self, rhs: &Self) {
-                assert_ulps_eq!(lhs, rhs, epsilon = EPS_FACTOR * V::epsilon(), max_ulps = ULPS);
+                match (lhs.is_nan(), rhs.is_nan()) {
+                    (true, true) => {}
+                    _ => {
+                        assert_ulps_eq!(lhs, rhs, epsilon = EPS_FACTOR * V::epsilon(),
+                            max_ulps = ULPS);
+                    }
+                }
+            }
+
+            /// Exactly compare `lhs` and `rhs` and return the result.
+            fn eq(lhs: &Self, rhs: &Self) -> bool {
+                (lhs.is_nan() && rhs.is_nan())
+                    || lhs == rhs
             }
 
+            /// Approximately compare `lhs` and `rhs` for floating point types or exactly compare
+            /// for other types and return the result.
             fn approx_eq(lhs: &Self, rhs: &Self) -> bool {
-                ulps_eq!(lhs, rhs, epsilon = EPS_FACTOR * V::epsilon(), max_ulps = ULPS)
+                (lhs.is_nan() && rhs.is_nan())
+                    || ulps_eq!(lhs, rhs, epsilon = EPS_FACTOR * V::epsilon(), max_ulps = ULPS)
             }
         }
     }
@@ -140,32 +165,25 @@ impl<V> crate::lib::ops::Deref for A<V> {
 
 mod a_struct {
     storage_types! {
-        // Quickcheck 0.8 required for i128/u128 support. Use PrimInt after upgrade.
-        types: Float, usize, u8, u16, u32, u64, isize, i8, i16, i32, i64;
+        types: Float, PrimInt;
 
         use super::super::A;
 
         impl quickcheck::Arbitrary for A<V> {
-            fn arbitrary<G>(g: &mut G) -> Self
-            where
-                G: quickcheck::Gen,
-            {
+            fn arbitrary(g: &mut quickcheck::Gen) -> Self {
                 A { v: V::arbitrary(g), }
             }
         }
     }
 
     storage_types! {
-        types: BigInt, BigUint, Ratio, i128, u128;
+        types: BigInt, BigUint, Ratio;
 
         use crate::num::FromPrimitive;
         use super::super::A;
 
         impl quickcheck::Arbitrary for A<V> {
-            fn arbitrary<G>(g: &mut G) -> Self
-            where
-                G: quickcheck::Gen,
-            {
+            fn arbitrary(g: &mut quickcheck::Gen) -> Self {
                 A {
                     v: loop {
                         let v = V::from_f64(<f64 as quickcheck::Arbitrary>::arbitrary(g));