Correct Quantity::new to be zero-cost for float storage types.

[iliekturtles]

Use a default constant of -0.0 to allow for floating point
optimizations. For a value, v: Float, adding -0.0 is a no-op while
adding 0.0 will change the sign if v is -0.0. Resolves #143.

   v
 0.0 + -0.0 =  0.0
-0.0 +  0.0 =  0.0 # v + 0.0 != v
-0.0 + -0.0 = -0.0

@raimundomartins with the changes in this PR your example program generates the expected zero-cost LLVM-IR! If you could review / test with your full code that would be greatly appreciated.

; call uom_opt_test::value
%0 = tail call fastcc double @_ZN12uom_opt_test5value17h5bf2d6ab643b092cE(double 1.111100e+04)
; call uom_opt_test::value
%1 = tail call fastcc double @_ZN12uom_opt_test5value17h5bf2d6ab643b092cE(double 2.222200e+04)
; call uom_opt_test::half_way
%2 = tail call fastcc double @_ZN12uom_opt_test8half_way17h905d0903778e0b79E(double %0)
; call uom_opt_test::half_way
%3 = tail call fastcc double @_ZN12uom_opt_test8half_way17h905d0903778e0b79E(double %1)

[raimundomartins]

My "full code" does not omit the add 0.0, but I'm unable to pinpoint why, since it's just the following benchmark tests:

    #[bench]
    fn compare_uom_trigonometric_with_direct(b: &mut Bencher) {
        b.iter(||
            (0..1000).map(|i| i as f64).
            fold(0.0, |old, new| {
                old + new.sin().sin()
            })
        )
    }
    #[bench]
    fn uom_trigonometric(b: &mut Bencher) {
        use uom_ops::Trigonometric;
        b.iter(||
            (0..1000).map(|i| i as f64).
            fold(0.0, |old, new| {
                old + Angle::new::<radian>(new.sin()).sin().get::<ratio>()
            })
        )
    }

with uom_ops::Trigonometric being:

pub trait Trigonometric<Result> {
    fn sin(&self) -> Result;
    fn cos(&self) -> Result;
    fn tan(&self) -> Result;
}
macro_rules! impl_si_trait {
    {$trait:ident, ($input:ident, $input_unit:ty), ($output:ident, $output_unit:ty): [$($func:ident),+]$(, {$($extra:tt)*})?} => {
        impl<T> $trait<$output<SI<T>, T>> for $input<SI<T>, T>
        where
            T: uom::num::Float + uom::Conversion<T>,
            radian: uom::Conversion<T, T = <T as uom::Conversion<T>>::T>,
            ratio: uom::Conversion<T, T = <T as uom::Conversion<T>>::T>,
        {
            $(fn $func(&self) -> $output<SI<T>, T> {
                $output::<SI<T>, T>::new::<$output_unit>(self.get::<$input_unit>().$func())
            })+
            $($($extra)*)?
        }
    };
}

impl_si_trait!{Trigonometric, (Angle, radian), (Ratio, ratio): [sin, cos, tan]} }

I fail to see why it doesn't vanish here but vanishes in the previous example program. I did make sure I was Compiling uom v0.23.1 (https://github.com/iliekturtles/uom/?rev=ea778679fa7e38156a97538dca48ab2a999f7a37#ea778679)

Now a bit off-topic, would you be interested to collaborate on a crate which impl's functions similar to the above? Surely there's a better implementation for it than this, and I find these functions to be of such basic use to require users to extract a value, use the op, and convert to another value. (I actually believe it removes all value of the uom crate itself). If so, how do you wish to proceed, by opening an issue?
Another sidenote, I use SI<T>, T because I think it would be great that this crate could be used with vectors and matrices.

[iliekturtles]

I'll checkout those benchmark examples this weekend and see if I can reproduce / solve the issue.

For sin, cos, tan, and other trigonometric functions I think it would be appropriate to put the functions directly into uom without the need to rely on a separate crate. I haven't had a chance to add a new issue for this to list out all the functions to implement but will do so this weekend if you don't get to it first. See length where hypot() is explicitly implemented.

uom/src/si/length.rs

Lines 67 to 82 in c02d306

	impl<U, V> Length<U, V>
	where
	U: super::Units<V> + ?Sized,
	V: ::num::Float + ::Conversion<V>,
	{
	/// Calculates the length of the hypotenuse of a right-angle triangle given the legs.
	#[cfg(feature = "std")]
	#[inline(always)]
	pub fn hypot(self, other: Self) -> Self {
	Length {
	dimension: ::lib::marker::PhantomData,
	units: ::lib::marker::PhantomData,
	value: self.value.hypot(other.value)
	}
	}
	}

[iliekturtles]

I ended up being busy all weekend and will try to review this week.

[iliekturtles]

I found the issue. to_base does + N::constant() so adding -0.0 becomes a no-op. from_base does - N::constant() and all of of a sudden that -0.0 becomes 0.0 and is no longer a no-op! All the original PR did was move the issue from to_base to from_base!

uom/src/system.rs

Lines 284 to 286 in c02d306

	(v.into_conversion() $(* U::$name::coefficient().powi(D::$symbol::to_i32()))+
	/ N::coefficient() - N::constant())
	.value()

uom/src/system.rs

Lines 302 to 304 in c02d306

	((v.into_conversion() + N::constant()) * N::coefficient()
	/ (V::coefficient() $(* U::$name::coefficient().powi(D::$symbol::to_i32()))+))
	.value()

So far I have two thoughts about how to fix the issue. Neither are great so I welcome suggestions.

1. Change constant() into two functions: additive_constant and subtractive_constant. Each version can return an appropriately signed 0.0.
2. Change constant() to take a parameter suggesting how the constant will be used. The function can now return the appropriate constant.

@raimundomartins I've currently gone with the second option. The updated code causes llvm to optimize the benchmarks down to a single llvm-ir function definition. If you could review the code and double check that the optimization is still happening that would be greatly appreciated.

[raimundomartins]

I had thought of that, but some quick tests didn't point to the opposite operation to not being optimized, so I thought that - -0.0 was being used.
Anyway, I'm glad to report that in my case these operations are zero-cost!
I just thought I should mention that the default impl of Conversion<T> should probably get the match arms as well.

uom/src/lib.rs

Lines 404 to 406 in 32d36e2

	fn constant(op: ConstantOp) -> Self::T {
	<Self::T as num::Zero>::zero()
	}

[iliekturtles]

Glad to hear it's fixed this time!

For the default implementation of fn constant I had started doing that last night but was a bit rushed and didn't pursue it. I was concerned how it would interact when T was an unsigned type. I'm not sure if the default implementation is actually used anywhere so I may just remove it. I'll merge the PR as soon as I have that resolved.

[iliekturtles]

Merged! I'll try to get a new version published in the next couple days!

[iliekturtles]

v0.24.0 published! Thanks again for submitting the issue.