Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AngleKind errors when multiplying by f64s #138

Closed
dunmatt opened this issue May 12, 2019 · 4 comments · Fixed by #139
Closed

AngleKind errors when multiplying by f64s #138

dunmatt opened this issue May 12, 2019 · 4 comments · Fixed by #139

Comments

@dunmatt
Copy link

dunmatt commented May 12, 2019

It looks like we missed something:

let compiles_fine = 5.0 * Angle::new::<radian>(1.0);
let wont_compile: Angle = 5.0 * Angle::new::<radian>(1.0);
let hacky: Angle = (5.0 * Angle::new::<radian>(1.0)).into();

throws Kind mismatch errors, apparently floats times quantities always returns a regular Kind rather than AngleKind. I'm going to look for a fix, if you don't see a PR in the next day or two assume that I got stuck somewhere and could use a hint.

@iliekturtles
Copy link
Owner

Issue is that the impl $MulDiv<Quantity> for V doesn't maintain the kind when calculating the new dimension. Maintaining the kind is simple but does it make sense for all cases?

// Will work as expected
let a: Angle = 5.0 * Angle::new::<radian>(1.0);

// Should `b` have a kind of `AngleKind`?
let b = 5.0 / Angle::new::<radian>(1.0);

Patch to fix issue:

diff --git a/src/system.rs b/src/system.rs
index fa0122c..87a8a04 100644
--- a/src/system.rs
+++ b/src/system.rs
@@ -510,7 +510,8 @@ macro_rules! system {
                                 $quantities<
                                     $($crate::typenum::$AddSubAlias<
                                       $crate::typenum::Z0,
-                                      D::$symbol>,)+>,
+                                      D::$symbol>,)+
+                                    D::Kind>,
                                 U, V>;

                             #[inline(always)]

@dunmatt
Copy link
Author

dunmatt commented May 13, 2019

I think so, because

let c = Angle::new::<radian>(1.0) / 5.0;

had better be an Angle, and b ought to be its reciprocal.

@iliekturtles
Copy link
Owner

impl $MulDiv<V> Quantity already maintains the kind:

uom/src/system.rs

Lines 464 to 471 in def7d69

impl<D, U, V> $crate::lib::ops::$MulDivTrait<V> for Quantity<D, U, V>
where
D: Dimension + ?Sized,
D::Kind: $crate::marker::$MulDivTrait,
U: Units<V> + ?Sized,
V: $crate::num::Num + $crate::Conversion<V>,
{
type Output = Quantity<D, U, V>;

I'll get the patch above committed and merged once the build completes. I'll also see about releasing a new version today or tomorrow before I lose internet access for a bit.

iliekturtles added a commit that referenced this issue May 13, 2019
`1.0 * Q` and `1.0 / Q` now both maintain Q's kind in the result. `Q *
1.0` and `Q / 1.0` already maintain the kind. Resolves #138.
@dunmatt
Copy link
Author

dunmatt commented May 13, 2019

Awesome, thanks!

iliekturtles added a commit that referenced this issue May 13, 2019
`1.0 * Q` and `1.0 / Q` now both maintain Q's kind in the result. `Q *
1.0` and `Q / 1.0` already maintain the kind. Resolves #138.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants