Use f{32|64}::powi instead of Typenum::Pow::powi.
#193
Conversation
|
I understand this is the faster solution, but I do wonder whether comments like https://docs.rs/typenum/1.12.0/src/typenum/type_operators.rs.html#103 still apply. Meaning that we should probably fix this in the |
To answer my own question, it does seem to apply indeed, i.e. |
src/system.rs
Outdated
| Quantity { | ||
| dimension: $crate::lib::marker::PhantomData, | ||
| units: $crate::lib::marker::PhantomData, | ||
| value: $crate::typenum::Pow::powi(self.value, e), | ||
| value: self.value.into_conversion().powi(E::to_i32()).value(), |
There was a problem hiding this comment.
This is probably just a lack of understanding, but looking at e.g. the implementation of recip, why is the trait bound
V: $crate::num::Float
and the implementation
value: self.value.powi(E::to_i32()),
not applicable here?
There was a problem hiding this comment.
I thought I tested this when I did the original fix, but I must have been pretty distracted because I checked it again and it works. I just pushed the fix and am waiting on the build to finish. Thanks for the review!
In Rust 1.45 the upgrade to LLVM 10.0 changes the behavior of
`f{32|64}::powi` on Windows. This commit changes the `Quantity::powi`
implementation to simplify down to `f{32|64}::powi` instead of using
`Typenum::Pow::powi` which uses a less precise algorithm. Resolves #192.
In Rust 1.45 the upgrade to LLVM 10.0 changes the behavior of
f{32|64}::powion Windows. This commit changes theQuantity::powiimplementation to simplify down to
f{32|64}::powiinstead of usingTypenum::Pow::powiwhich uses a less precise algorithm. Resolves #192.Reviews welcome. Will plan to merge in a couple days.