Add option to skip renormalization of AngleAxis and Quat

[tkoolen]

I've noticed that the renormalization in AngleAxis now accounts for a nontrivial percentage of time spent in my kinematics code, even though I know for sure that the axis I pass in is a unit vector. In addition, there could be scalar types for which normalization is not desired (see JuliaRobotics/RigidBodyDynamics.jl#87). This PR adds the option to skip the normalization in the constructor.

I checked that the generated code is the same as it was before for calls to Quat(w, x, y, z) and AngleAxis(theta, x, y, z). This required an @inline annotation for the inner constructors.

[c42f]

Looks really useful to me.

[c42f]

Does this actually need to be a Val or can you just pass a normal Bool? I'd expect constant prop to optimize away the branches anyway at the LLVM level. The only time I'd expect this to make any difference is if the two sides of the branch could result in different types for T. But I'm not sure it's very useful to allow T == Int, say, in any case.

[tkoolen]

Good call; just tested: LLVM is indeed able to optimize that branch away. I'll update the PR.

[tkoolen]

Done.

[andyferris]

This is a good idea.

I'm impressed that LLVM can optimize the branch away optimally with a Bool. It's also been muted that we are very close to getting inter-function constant propagation with inlines, so inference will presumably do the job in v1.0.

[c42f]

Looks good to me.

[c42f]

Nice documentation, by the way :)

[tkoolen]

Thanks!

Re:

The only time I'd expect this to make any difference is if the two sides of the branch could result in different types for T.

The scalar type of the Quat/AngleAxis is currently forced to either be specified by the user (for the inner constructor) or to be promote_type applied to the input argument types (for the outer constructor), so the way things are set up right now, this will never cause type instability. However, without normalization, you can now actually create an AngleAxis{Int}, while this was impossible before since it would result in an InexactError (as it still does with normalization turned on). I don't think this matters too much.

BTW, both before and after this PR, the current constructor setup can result in moderately unexpected behavior for some Real types due to the Base method

sqrt(x::Real) = sqrt(float(x))

which, combined with the conversion from Float64 to Rational means that Quat(1//1, 1//2, 1//3, 1//4) results in

Quat{Rational{Int64}}(3774540503726565//4503599627370496, 3774540503726565//9007199254740992, 1258180167908855//4503599627370496, 3774540503726565//18014398509481984)

Again, I don't think this matters too much.

[c42f]

the things are set up right now, this will never cause type instability

Agreed, though I'm not sure our constructors make a huge amount of sense as they are right now. For example, I think the Quat constructor might be better written as

@inline Quat(w::T, x::T, y::T, z::T) where {T} = Quat{typeof((zero(T) + one(T))/one(T))}(w, x, y, z)
@inline Quat(w, x, y, z) = Quat(promote(w, x, y, z)...)

Which would correctly promote integers, rather than just producing an error.

Hmm. Does this affect the choice of Val-or-not?

[c42f]

By the way, feel free to merge, unless you want to explore the consequences of "fixing" the constructors.

[tkoolen]

OK, merged, since the constructor stuff is more or less orthogonal to this change (Rational behavior was present before as well).