[WIP] Add support for Complex number storage types. #287
Conversation
|
Oh, is it a problem if the types don't implement It seems to be a problem because Line 441 in fe7f0b3 Kinda crazy how some intuitive promises are broken by math. |
There was a problem hiding this comment.
I believe the PartialOrd issue can be resolved by using Complex<T>'s underlying type as the ConversionFactor<V> type. See the two suggestions in this review for the changes that I believe are needed. I haven't attempted to compile so additional changes are likely needed.
|
It may have worked (I'm still getting errors, but I think unrelated ones, I'll work on them after). Just to check my thought process, could you confirm/correct this (applies to
Also, I have no idea what that Thank you for your patience and help 🤗 |
|
It seems to work now btw :D |
|
Excellent! I started the build and will review in more detail later today. |
There was a problem hiding this comment.
tests::Test needs to be implemented. See the relevant code for f32/f64 below. You'll want to use similar approx comparisons for the real part. I assume you'll want approx comparisons for the imaginary part.
Lines 99 to 142 in fe7f0b3
|
I started some work. A lot of code duplication and still quite a few errors, but could you look over these points: |
|
Approved the build again so that you'll get test results. I'm hoping to have time tomorrow to do an in depth review. |
|
Yeah no it'll still error out, but I don't know why. I'm having this error (and 24 similar more): referencing this. ... but I have no idea what is going on here. If you can help, I'd greatly appreciate it. (Take your time :) ) |
|
Looks like the issue is that |
|
Despite my best intentions I just didn't get to doing a deeper review. Hopefully this weekend or early next week. |
|
no problem, I'm not in a hurry. |
|
Will address the last review later. Rebased, will squash sometime later too... |
There was a problem hiding this comment.
With the changes in this review and if you temporarily comment out all of the methods like is_normal that rely on Float/FloatCore the crate will compile and run tests! Some of the tests are failing and I haven't figured out what the solution for the Float/FloatCore is going to be yet.
I'm also noticing that a lot of the Complex storage_types! blocks are a copy-and-paste of the Float blocks with V replaced with VV. I don't want to block acceptance of this PR on reducing this duplicate code. Please post If you have any thoughts about the best way to reduce this duplication.
Add new features to extern crate approx; statement:
- #[cfg(all(test, any(feature = "f32", feature = "f64")))]
+ #[cfg(all(test, any(feature = "f32", feature = "f64", feature = "complex32", feature = "complex64")))]
#[macro_use]
extern crate approx;Add to src/tests/asserts.rs:
storage_types! {
types: Complex;
use super::*;
assert_impl_all!(Quantity<Q<Z0, Z0, Z0>, U<V>, V>:
Clone, Copy, Debug, PartialEq, Send, Sync, Unpin);
#[cfg(feature = "std")]
assert_impl_all!(Quantity<Q<Z0, Z0, Z0>, U<V>, V>:
RefUnwindSafe, UnwindSafe);
assert_not_impl_any!(Quantity<Q<Z0, Z0, Z0>, U<V>, V>:
Binary, Display, Eq, Hash, LowerExp, LowerHex, Octal, Ord, PartialOrd, UpperExp, UpperHex);
assert_impl_all!(QuantityArguments<Q<Z0, Z0, Z0>, U<V>, V, meter>:
Clone, Copy, Debug, Display, LowerExp, Send, Sync, Unpin, UpperExp);
#[cfg(feature = "std")]
assert_impl_all!(QuantityArguments<Q<Z0, Z0, Z0>, U<V>, V, meter>:
RefUnwindSafe, UnwindSafe);
assert_not_impl_any!(QuantityArguments<Q<Z0, Z0, Z0>, U<V>, V, meter>:
Binary, Eq, Hash, LowerHex, Octal, Ord, PartialEq, PartialOrd, UpperHex);
}
|
Ok, sorry I got a bit delayed on this, but all should be resolved now. I commented out the tests where the weird issues with And about the duplication with |
|
Excellent! I'll try to get to reviewing the latest changes this week. |
src/tests/system.rs
Outdated
| // Somehow Complex<f32|f64> don't implement these methods, even though they | ||
| // should, bc f32 / f64 implement Float and FloatCore: | ||
| // - https://docs.rs/num/latest/num/struct.Complex.html#impl-5 | ||
| // - https://docs.rs/num/latest/num/struct.Complex.html#impl-4 | ||
| // | ||
| // See discussion here: | ||
| // https://github.com/iliekturtles/uom/pull/287#pullrequestreview-922005933 | ||
| /* | ||
| mod complex { |
There was a problem hiding this comment.
I spent some time looking at this tonight and I believe what we'll need to do is explicitly implement these functions for f32, f64, Complex32, and Complex64 using the storage_types! macro. Complex<T> doesn't implement Float/FloatCore, but it does implement these methods when the underlying type, T, implements FloatCore.
storage_types! {
types: Float, Complex;
impl<...> Quantity<..., V /*V type alias defined by storage_types!*/> {
fn is_infinite(self) -> bool { self.value.is_infinite() }
}
}
// I don't think a separate storage_types! is necessary. The code should be able to be the same.
//storage_types! {
// types: Complex;
//
// impl<...> Quantity<..., V> {
// fn is_infinite(...) -> ... { ... }
// }
}There was a problem hiding this comment.
I don't completely understand why I should implement these methods on f32 and f64 too, num implements them for these two just fine, doesn't it?
As for doing it for the complexes... it's weird. Before the compiler was complaining that it didn't find an implementation for is_infinite et al., but now when I implement it with the following patch, it tells me it has duplicate implementation:
diff --git a/src/tests/system.rs b/src/tests/system.rs
index 1355f80..cad6de9 100644
--- a/src/tests/system.rs
+++ b/src/tests/system.rs
@@ -584,7 +584,6 @@ mod fixed {
//
// See discussion here:
// https://github.com/iliekturtles/uom/pull/287#pullrequestreview-922005933
-/*
mod complex {
storage_types! {
types: Complex;
@@ -593,6 +592,11 @@ mod complex {
Q!(crate::tests, V);
+ impl<D, U> Quantity<D, U, V /*V type alias defined by storage_types!*/> {
+ fn is_infinite(self) -> bool { self.value.is_infinite() }
+ fn is_finite(self) -> bool { self.value.is_finite() }
+ }
+
#[test]
fn fp_categories() {
let float_categories = vec![
@@ -724,7 +728,6 @@ mod complex {
}
}
}
-*/
mod non_complex {
storage_types! {
error[E0592]: duplicate definitions with name `is_infinite`
|
::: src/tests/system.rs:596:13
|
596 | fn is_infinite(self) -> bool { self.value.is_infinite() }
| ---------------------------- other definition for `is_infinite`
|
::: src/tests/mod.rs:55:1
|
55 | / system! {
56 | | quantities: Q {
57 | | length: meter, L;
58 | | mass: kilogram, M;
... |
65 | | }
66 | | }
| |_- in this macro invocation
--> src/system.rs:613:13
|
613 | / pub fn is_infinite(self) -> bool
614 | | where
615 | | V: $crate::num::Float,
| |______________________________________^ duplicate definitions for `is_infinite`
|
= note: this error originates in the macro `system` (in Nightly builds, run with -Z macro-backtrace for more info)
Maybe I am also implementing it at the wrong location?
There was a problem hiding this comment.
I think I see where the duplicates are and will take a look in the next few days!
|
We're getting closer. I got all the tests compiling by moving the general function implementations diff --git a/src/system.rs b/src/system.rs
index 6324553..016f714 100644
--- a/src/system.rs
+++ b/src/system.rs
@@ -596,47 +596,6 @@ macro_rules! system {
U: Units<V> + ?Sized,
V: $crate::num::Num + $crate::Conversion<V>,
{
- /// Returns `true` if this value is `NAN` and `false` otherwise.
- #[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))]
- #[inline(always)]
- pub fn is_nan(self) -> bool
- where
- V: $crate::num::Float,
- {
- self.value.is_nan()
- }
-
- /// Returns `true` if this value is positive infinity or negative infinity and
- /// `false` otherwise.
- #[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))]
- #[inline(always)]
- pub fn is_infinite(self) -> bool
- where
- V: $crate::num::Float,
- {
- self.value.is_infinite()
- }
-
- /// Returns `true` if this number is neither infinite nor `NAN`.
- #[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))]
- #[inline(always)]
- pub fn is_finite(self) -> bool
- where
- V: $crate::num::Float,
- {
- self.value.is_finite()
- }
-
- /// Returns `true` if the number is neither zero, infinite, subnormal, or `NAN`.
- #[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))]
- #[inline(always)]
- pub fn is_normal(self) -> bool
- where
- V: $crate::num::Float,
- {
- self.value.is_normal()
- }
-
/// Returns the floating point category of the number. If only one property is
/// going to be tested, it is generally faster to use the specific predicate
/// instead.
@@ -649,43 +608,6 @@ macro_rules! system {
}
std! {
- /// Takes the cubic root of a number.
- ///
- #[cfg_attr(all(feature = "si", feature = "f32"), doc = " ```rust")]
- #[cfg_attr(not(all(feature = "si", feature = "f32")), doc = " ```rust,ignore")]
- /// # use uom::si::f32::*;
- /// # use uom::si::volume::cubic_meter;
- /// let l: Length = Volume::new::<cubic_meter>(8.0).cbrt();
- /// ```
- ///
- /// The input type must have dimensions divisible by three:
- ///
- #[cfg_attr(all(feature = "si", feature = "f32"), doc = " ```rust,compile_fail")]
- #[cfg_attr(not(all(feature = "si", feature = "f32")), doc = " ```rust,ignore")]
- /// # use uom::si::f32::*;
- /// # use uom::si::area::square_meter;
- /// // error[E0271]: type mismatch resolving ...
- /// let r = Area::new::<square_meter>(8.0).cbrt();
- /// ```
- #[inline(always)]
- pub fn cbrt(
- self
- ) -> Quantity<
- $quantities<$($crate::typenum::PartialQuot<D::$symbol, $crate::typenum::P3>),+>,
- U, V>
- where
- $(D::$symbol: $crate::lib::ops::PartialDiv<$crate::typenum::P3>,
- <D::$symbol as $crate::lib::ops::PartialDiv<$crate::typenum::P3>>::Output: $crate::typenum::Integer,)+
- D::Kind: $crate::marker::Div,
- V: $crate::num::Float,
- {
- Quantity {
- dimension: $crate::lib::marker::PhantomData,
- units: $crate::lib::marker::PhantomData,
- value: self.value.cbrt(),
- }
- }
-
autoconvert! {
/// Calculates the length of the hypotenuse of a right-angle triangle given the legs.
#[inline(always)]
@@ -769,39 +691,6 @@ macro_rules! system {
self.value.is_sign_negative()
}
- std! {
- /// Fused multiply-add. Computes `(self * a) + b` with only one rounding error.
- /// This produces a more accurate result with better performance than a separate
- /// multiplication operation followed by an add.
- ///
- /// ## Generic Parameters
- /// * `Da`: Dimension for parameter `a`.
- /// * `Ua`: Base units for parameter `a`.
- /// * `Ub`: Base units for parameter `b`.
- #[inline(always)]
- pub fn mul_add<Da, Ua, Ub>(
- self,
- a: Quantity<Da, Ua, V>,
- b: Quantity<$quantities<$($crate::typenum::Sum<D::$symbol, Da::$symbol>),+>, Ub, V>,
- ) -> Quantity<$quantities<$($crate::typenum::Sum<D::$symbol, Da::$symbol>),+>, U, V>
- where
- $(D::$symbol: $crate::lib::ops::Add<Da::$symbol>,
- <D::$symbol as $crate::lib::ops::Add<Da::$symbol>>::Output: $crate::typenum::Integer,)+
- D::Kind: $crate::marker::Mul,
- V: $crate::num::Float,
- Da: Dimension + ?Sized,
- Da::Kind: $crate::marker::Mul,
- Ua: Units<V> + ?Sized,
- Ub: Units<V> + ?Sized,
- {
- // (self * a) + b
- Quantity {
- dimension: $crate::lib::marker::PhantomData,
- units: $crate::lib::marker::PhantomData,
- value: self.value.mul_add(a.value, b.value),
- }
- }}
-
/// Takes the reciprocal (inverse) of a number, `1/x`.
///
#[cfg_attr(all(feature = "si", feature = "f32"), doc = " ```rust")]
@@ -827,76 +716,6 @@ macro_rules! system {
}
}
- std! {
- /// Raises a quantity to an integer power.
- ///
- #[cfg_attr(all(feature = "si", feature = "f32"), doc = " ```rust")]
- #[cfg_attr(not(all(feature = "si", feature = "f32")), doc = " ```rust,ignore")]
- /// # use uom::si::f32::*;
- /// # use uom::si::length::meter;
- /// use uom::typenum::P2;
- ///
- /// let a: Area = Length::new::<meter>(3.0).powi(P2::new());
- /// ```
- ///
- /// ## Generic Parameters
- /// * `E`: `typenum::Integer` power.
- #[inline(always)]
- pub fn powi<E>(
- self, _e: E
- ) -> Quantity<$quantities<$($crate::typenum::Prod<D::$symbol, E>),+>, U, V>
- where
- $(D::$symbol: $crate::lib::ops::Mul<E>,
- <D::$symbol as $crate::lib::ops::Mul<E>>::Output: $crate::typenum::Integer,)+
- D::Kind: $crate::marker::Mul,
- E: $crate::typenum::Integer,
- V: $crate::num::Float,
- {
- Quantity {
- dimension: $crate::lib::marker::PhantomData,
- units: $crate::lib::marker::PhantomData,
- value: self.value.powi(E::to_i32()),
- }
- }
-
- /// Takes the square root of a number. Returns `NAN` if `self` is a negative
- /// number.
- ///
- #[cfg_attr(all(feature = "si", feature = "f32"), doc = " ```rust")]
- #[cfg_attr(not(all(feature = "si", feature = "f32")), doc = " ```rust,ignore")]
- /// # use uom::si::f32::*;
- /// # use uom::si::area::square_meter;
- /// let l: Length = Area::new::<square_meter>(4.0).sqrt();
- /// ```
- ///
- /// The input type must have dimensions divisible by two:
- ///
- #[cfg_attr(all(feature = "si", feature = "f32"), doc = " ```rust,compile_fail")]
- #[cfg_attr(not(all(feature = "si", feature = "f32")), doc = " ```rust,ignore")]
- /// # use uom::si::f32::*;
- /// # use uom::si::length::meter;
- /// // error[E0271]: type mismatch resolving ...
- /// let r = Length::new::<meter>(4.0).sqrt();
- /// ```
- #[inline(always)]
- pub fn sqrt(
- self
- ) -> Quantity<
- $quantities<$($crate::typenum::PartialQuot<D::$symbol, $crate::typenum::P2>),+>,
- U, V>
- where
- $(D::$symbol: $crate::typenum::PartialDiv<$crate::typenum::P2>,
- <D::$symbol as $crate::typenum::PartialDiv<$crate::typenum::P2>>::Output: $crate::typenum::Integer,)+
- D::Kind: $crate::marker::Div,
- V: $crate::num::Float,
- {
- Quantity {
- dimension: $crate::lib::marker::PhantomData,
- units: $crate::lib::marker::PhantomData,
- value: self.value.sqrt(),
- }
- }}
-
/// Returns the maximum of the two quantities.
#[inline(always)]
pub fn max(self, other: Self) -> Self
@@ -924,6 +743,194 @@ macro_rules! system {
}
}
+ // Explicitly definte floating point methods for float and complex storage types.
+ // `Complex<T>` doesn't implement `Float`/`FloatCore`, but it does implement these methods
+ // when the underlying type, `T`, implements `FloatCore`.
+ mod float {
+ storage_types! {
+ types: Float, Complex;
+
+ use super::super::*;
+
+ impl<D, U> Quantity<D, U, V>
+ where
+ D: Dimension + ?Sized,
+ U: Units<V> + ?Sized,
+ {
+ /// Returns `true` if this value is `NAN` and `false` otherwise.
+ #[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))]
+ #[inline(always)]
+ pub fn is_nan(self) -> bool
+ {
+ self.value.is_nan()
+ }
+
+ /// Returns `true` if this value is positive infinity or negative infinity and
+ /// `false` otherwise.
+ #[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))]
+ #[inline(always)]
+ pub fn is_infinite(self) -> bool
+ {
+ self.value.is_infinite()
+ }
+
+ /// Returns `true` if this number is neither infinite nor `NAN`.
+ #[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))]
+ #[inline(always)]
+ pub fn is_finite(self) -> bool
+ {
+ self.value.is_finite()
+ }
+
+ /// Returns `true` if the number is neither zero, infinite, subnormal, or `NAN`.
+ #[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))]
+ #[inline(always)]
+ pub fn is_normal(self) -> bool
+ {
+ self.value.is_normal()
+ }
+
+ std! {
+ /// Takes the cubic root of a number.
+ ///
+ #[cfg_attr(all(feature = "si", feature = "f32"), doc = " ```rust")]
+ #[cfg_attr(not(all(feature = "si", feature = "f32")), doc = " ```rust,ignore")]
+ /// # use uom::si::f32::*;
+ /// # use uom::si::volume::cubic_meter;
+ /// let l: Length = Volume::new::<cubic_meter>(8.0).cbrt();
+ /// ```
+ ///
+ /// The input type must have dimensions divisible by three:
+ ///
+ #[cfg_attr(all(feature = "si", feature = "f32"), doc = " ```rust,compile_fail")]
+ #[cfg_attr(not(all(feature = "si", feature = "f32")), doc = " ```rust,ignore")]
+ /// # use uom::si::f32::*;
+ /// # use uom::si::area::square_meter;
+ /// // error[E0271]: type mismatch resolving ...
+ /// let r = Area::new::<square_meter>(8.0).cbrt();
+ /// ```
+ #[inline(always)]
+ pub fn cbrt(
+ self
+ ) -> Quantity<
+ $quantities<$($crate::typenum::PartialQuot<D::$symbol, $crate::typenum::P3>),+>,
+ U, V>
+ where
+ $(D::$symbol: $crate::lib::ops::PartialDiv<$crate::typenum::P3>,
+ <D::$symbol as $crate::lib::ops::PartialDiv<$crate::typenum::P3>>::Output: $crate::typenum::Integer,)+
+ D::Kind: $crate::marker::Div,
+ {
+ Quantity {
+ dimension: $crate::lib::marker::PhantomData,
+ units: $crate::lib::marker::PhantomData,
+ value: self.value.cbrt(),
+ }
+ }
+
+ /// Fused multiply-add. Computes `(self * a) + b` with only one rounding error.
+ /// This produces a more accurate result with better performance than a separate
+ /// multiplication operation followed by an add.
+ ///
+ /// ## Generic Parameters
+ /// * `Da`: Dimension for parameter `a`.
+ /// * `Ua`: Base units for parameter `a`.
+ /// * `Ub`: Base units for parameter `b`.
+ #[inline(always)]
+ pub fn mul_add<Da, Ua, Ub>(
+ self,
+ a: Quantity<Da, Ua, V>,
+ b: Quantity<$quantities<$($crate::typenum::Sum<D::$symbol, Da::$symbol>),+>, Ub, V>,
+ ) -> Quantity<$quantities<$($crate::typenum::Sum<D::$symbol, Da::$symbol>),+>, U, V>
+ where
+ $(D::$symbol: $crate::lib::ops::Add<Da::$symbol>,
+ <D::$symbol as $crate::lib::ops::Add<Da::$symbol>>::Output: $crate::typenum::Integer,)+
+ D::Kind: $crate::marker::Mul,
+ Da: Dimension + ?Sized,
+ Da::Kind: $crate::marker::Mul,
+ Ua: Units<V> + ?Sized,
+ Ub: Units<V> + ?Sized,
+ {
+ #[allow(unused_imports)]
+ use $crate::num_traits::MulAdd;
+
+ // (self * a) + b
+ Quantity {
+ dimension: $crate::lib::marker::PhantomData,
+ units: $crate::lib::marker::PhantomData,
+ value: self.value.mul_add(a.value, b.value),
+ }
+ }
+
+ /// Raises a quantity to an integer power.
+ ///
+ #[cfg_attr(all(feature = "si", feature = "f32"), doc = " ```rust")]
+ #[cfg_attr(not(all(feature = "si", feature = "f32")), doc = " ```rust,ignore")]
+ /// # use uom::si::f32::*;
+ /// # use uom::si::length::meter;
+ /// use uom::typenum::P2;
+ ///
+ /// let a: Area = Length::new::<meter>(3.0).powi(P2::new());
+ /// ```
+ ///
+ /// ## Generic Parameters
+ /// * `E`: `typenum::Integer` power.
+ #[inline(always)]
+ pub fn powi<E>(
+ self, _e: E
+ ) -> Quantity<$quantities<$($crate::typenum::Prod<D::$symbol, E>),+>, U, V>
+ where
+ $(D::$symbol: $crate::lib::ops::Mul<E>,
+ <D::$symbol as $crate::lib::ops::Mul<E>>::Output: $crate::typenum::Integer,)+
+ D::Kind: $crate::marker::Mul,
+ E: $crate::typenum::Integer,
+ {
+ Quantity {
+ dimension: $crate::lib::marker::PhantomData,
+ units: $crate::lib::marker::PhantomData,
+ value: self.value.powi(E::to_i32()),
+ }
+ }
+
+ /// Takes the square root of a number. Returns `NAN` if `self` is a negative
+ /// number.
+ ///
+ #[cfg_attr(all(feature = "si", feature = "f32"), doc = " ```rust")]
+ #[cfg_attr(not(all(feature = "si", feature = "f32")), doc = " ```rust,ignore")]
+ /// # use uom::si::f32::*;
+ /// # use uom::si::area::square_meter;
+ /// let l: Length = Area::new::<square_meter>(4.0).sqrt();
+ /// ```
+ ///
+ /// The input type must have dimensions divisible by two:
+ ///
+ #[cfg_attr(all(feature = "si", feature = "f32"), doc = " ```rust,compile_fail")]
+ #[cfg_attr(not(all(feature = "si", feature = "f32")), doc = " ```rust,ignore")]
+ /// # use uom::si::f32::*;
+ /// # use uom::si::length::meter;
+ /// // error[E0271]: type mismatch resolving ...
+ /// let r = Length::new::<meter>(4.0).sqrt();
+ /// ```
+ #[inline(always)]
+ pub fn sqrt(
+ self
+ ) -> Quantity<
+ $quantities<$($crate::typenum::PartialQuot<D::$symbol, $crate::typenum::P2>),+>,
+ U, V>
+ where
+ $(D::$symbol: $crate::typenum::PartialDiv<$crate::typenum::P2>,
+ <D::$symbol as $crate::typenum::PartialDiv<$crate::typenum::P2>>::Output: $crate::typenum::Integer,)+
+ D::Kind: $crate::marker::Div,
+ {
+ Quantity {
+ dimension: $crate::lib::marker::PhantomData,
+ units: $crate::lib::marker::PhantomData,
+ value: self.value.sqrt(),
+ }
+ }}
+ }
+ }
+ }
+
impl<D, U, V> $crate::lib::clone::Clone for Quantity<D, U, V>
where
D: Dimension + ?Sized,
diff --git a/src/tests/system.rs b/src/tests/system.rs
index 1355f80..459c9c6 100644
--- a/src/tests/system.rs
+++ b/src/tests/system.rs
@@ -577,14 +577,6 @@ mod fixed {
}
}
-// Somehow Complex<f32|f64> don't implement these methods, even though they
-// should, bc f32 / f64 implement Float and FloatCore:
-// - https://docs.rs/num/latest/num/struct.Complex.html#impl-5
-// - https://docs.rs/num/latest/num/struct.Complex.html#impl-4
-//
-// See discussion here:
-// https://github.com/iliekturtles/uom/pull/287#pullrequestreview-922005933
-/*
mod complex {
storage_types! {
types: Complex;
@@ -595,7 +587,7 @@ mod complex {
#[test]
fn fp_categories() {
- let float_categories = vec![
+ let float_categories = [
VV::neg_infinity(),
VV::min_value(),
VV::zero(),
@@ -603,12 +595,13 @@ mod complex {
VV::infinity(),
VV::nan(),
];
- for re in float_categories.clone() {
+
+ for re in float_categories {
for im in float_categories {
let complex = V::new(re, im);
// Infinities
- if re.is_infinite() || im.is_infinite() {
+ if complex.is_infinite() {
assert!(!Length::new::<meter>(complex).is_finite());
assert!(Length::new::<meter>(complex).is_infinite());
assert!(!Length::new::<meter>(complex).is_normal());
@@ -616,7 +609,7 @@ mod complex {
}
// NaNs
- if re.is_nan() || im.is_nan() {
+ if complex.is_nan() {
assert!(!Length::new::<meter>(complex).is_finite());
assert!(!Length::new::<meter>(complex).is_infinite());
assert!(!Length::new::<meter>(complex).is_normal());
@@ -624,8 +617,7 @@ mod complex {
}
// Finite and normal numbers
- if (!re.is_infinite() && !re.is_nan())
- && (!im.is_infinite() && !im.is_nan()) {
+ if !complex.is_infinite() && !complex.is_nan() {
assert!(Length::new::<meter>(complex).is_finite());
assert!(!Length::new::<meter>(complex).is_infinite());
assert!(Length::new::<meter>(complex).is_normal());
@@ -638,26 +630,22 @@ mod complex {
quickcheck! {
#[allow(trivial_casts)]
fn is_nan(v: A<V>) -> bool {
- v.re.is_nan() || v.im.is_nan()
- == Length::new::<meter>(*v).is_nan()
+ v.is_nan() == Length::new::<meter>(*v).is_nan()
}
#[allow(trivial_casts)]
fn is_infinite(v: A<V>) -> bool {
- v.re.is_infinite() || v.im.is_infinite()
- == Length::new::<meter>(*v).is_infinite()
+ v.is_infinite() == Length::new::<meter>(*v).is_infinite()
}
#[allow(trivial_casts)]
fn is_finite(v: A<V>) -> bool {
- v.re.is_finite() && v.im.is_finite()
- == Length::new::<meter>(*v).is_finite()
+ v.is_finite() == Length::new::<meter>(*v).is_finite()
}
#[allow(trivial_casts)]
fn is_normal(v: A<V>) -> bool {
- v.re.is_normal() && v.im.is_normal()
- == Length::new::<meter>(*v).is_normal()
+ v.is_normal() == Length::new::<meter>(*v).is_normal()
}
#[cfg(feature = "std")]
@@ -672,16 +660,12 @@ mod complex {
Test::eq(&v.cbrt(), &l.value)
}
- #[cfg(feature = "std")]
- #[allow(trivial_casts)]
- fn hypot(l: A<V>, r: A<V>) -> bool {
- Test::eq(&Length::new::<meter>(l.hypot(*r)),
- &Length::new::<meter>(*l).hypot(Length::new::<meter>(*r)))
- }
-
#[cfg(feature = "std")]
#[allow(trivial_casts)]
fn mul_add(s: A<V>, a: A<V>, b: A<V>) -> bool {
+ #[allow(unused_imports)]
+ use num_traits::MulAdd;
+
let r: Quantity<Q<P2, Z0, Z0>, U<V>, V> = Length::new::<meter>(*s).mul_add(
Length::new::<meter>(*a),
Quantity::<Q<P2, Z0, Z0>, U<V>, V> {
@@ -693,17 +677,6 @@ mod complex {
Test::eq(&s.mul_add(*a, *b), &r.value)
}
- #[allow(trivial_casts)]
- fn recip(v: A<V>) -> bool {
- let a: Quantity<Q<N1, Z0, Z0>, U<V>, V> = Quantity::<Q<P1, Z0, Z0>, U<V>, V> {
- dimension: PhantomData,
- units: PhantomData,
- value: *v,
- }.recip();
-
- Test::eq(&v.recip(), &a.value)
- }
-
#[cfg(feature = "std")]
#[allow(trivial_casts)]
fn powi(v: A<V>) -> bool {
@@ -724,7 +697,6 @@ mod complex {
}
}
}
-*/
mod non_complex {
storage_types! {
diff --git a/src/unit.rs b/src/unit.rs
index c83f5c9..cf86fc7 100644
--- a/src/unit.rs
+++ b/src/unit.rs
@@ -255,15 +255,16 @@ macro_rules! unit {
type T = VV;
#[inline(always)]
- #[allow(trivial_numeric_casts)]
+ #[allow(clippy::inconsistent_digit_grouping)]
fn coefficient() -> Self::T {
- unit!(@coefficient $($conversion),+) as Self::T
+ unit!(@coefficient $($conversion),+)
}
#[inline(always)]
- #[allow(unused_variables, trivial_numeric_casts)]
+ #[allow(unused_variables)]
+ #[allow(clippy::inconsistent_digit_grouping)]
fn constant(op: $crate::ConstantOp) -> Self::T {
- unit!(@constant op $($conversion),+) as Self::T
+ unit!(@constant op $($conversion),+)
}
}
|
|
I still didn't really get why it works in the I'll take a look at it later... |
The issue is that the original code implemented I also reviewed the test failures. The issue is that Other underlying storage types have similar issues (#44) so I wont block inclusion of complex. . If you believe there are no other open issues, will you squash your commits into a single commit (or a few that each make specific changes). I'll do a final review and believe we can accept. |
|
Sorry for the delay, finals prep claimed most of my time the last few weeks.
Oh, I see.
Oh, yeah, would've never found that. Thank you! Well then, I guess we'll include them. Should I remove/comment out the tests to make the pipeline succeed? Thank you a lot for your support on this PR! |
|
Merged! Thank you so much for the PR and for your patience as I found time to work through the review. No need to comment out the tests as they aren't run by default. Currently I'm only requiring full test coverage for I'll likely look to do a new release to crates.io in the next couple weeks. |
I have no idea what I'm doing.
Based on e2d5f20. Addresses #286.
Todo:
Discuss before merge:
Complex<T>, or maybe onlyComplex<T>instead ofComplex32,Complex64?