swift-numerics
swift-numerics copied to clipboard
Rename halfAngle to argument and make it public
As of now, Quaternion
s provide a number of different representations to access the encoded rotational/transformational properties, alongside a function to directly act on (rotate) an arbitrary 3 dimensional vector. Two of these representations are angle-axis and polar coordinate form. Both of them are defined, not just for unit quaternions, but for any arbitrary finite and nonzero quaternion and they both have very similar attributes, i.e.: they both are defined by the length
of the quaternion, the axis
of rotation and an angle alongside the axis. However, the definiton of these angles is different for polar (in [0, π] range) and angle-axis (in [0,2π] range), and while access to the angle of the angle-axis representation has been made publicly available via the angle
property, the (half)angle of the polar form has not been made publicly available to avoid unnecessary churn in the API surface (though it has been available internally to the module via halfAngle
). The only way to (publicly) access the angle of the polar form is through the phase
value of the polar
tuple property, which also yields length
and axis
.
This PR tries to address this issue, by renaming halfAngle
to argument
, and making it publicy available. Additionally, for consistency, it removes references to halfAngle
through phase
and replaces them with argument
. The later, however, does require 2 changes in the public API:
-
The polar initializer has been renamed:
public init(length: RealType, phase: RealType, axis: SIMD3<RealType>)
->public init(length: RealType, argument: RealType, axis: SIMD3<RealType>)
This could be solved by providing both, and deprecating the first one. -
The polar tuple has been changed (phase -> argument):
public var polar: (length: RealType, phase: RealType, axis: SIMD3<RealType>)
->public var polar: (length: RealType, argument: RealType, axis: SIMD3<RealType>)
This can not be solved by deprecation (AFAIK), and would be a source breaking change.
Given that quaternions are not yet part of the main branch, I have not yet addressed these 2 issues, and both would be source breaking. But I am definitely willing to add deprecations or change the PR if these source breaking changes should be addressed.
I think that this is contrary to the usual notions of argument
for quaternions--in particular, the argument of a quaternion should be something like the imaginary part of it's logarithm, rather than a real angle. Is your use of argument
precedented somewhere?
If it's not argument
, what do we call this? I might just use halfAngle
. Thoughts?
That is unfortunate. I was not aware of this notion, thanks for clarifying. I had hoped it would solve ambiguity, not introduce a new one (that is arguably worse).
Is your use of argument precedented somewhere?
I first found the usage of the wording in Real Quaternionic Calculus Handbook. It denotes the quaternion "argument" as the angle θ, where θ is 2π-periodic, and the the "principle argument" as the angle in range [0,π], where the quadrant must otherwise be specified or understood. I did the initial tests with principleArgument
but later switched to just calling it argument
for brevity, and denoted it as "principle argument" in the header documentation.
If it's not
argument
, what do we call this? I might just usehalfAngle
. Thoughts?
I do not mind using halfAngle
. It is intuitive, after all: it is "half the angle
", which is correct. It might give halfAngle
the attribute of being "dependent" on angle
though, which it is not. Also, halfAngle
is not that useful when dealing with Angle-Axis, just like axis
is not for polar form, and so I had hoped that, with the new naming it would move both of them into their own "categories". I realize, both this cons are fairly weak, but I had hoped to solve them nonetheless :)
Edit: If we were to continue with halfAngle
, I would still like to eliminate phase
from the public API. In this case, the issue with source breaking changes would persist though.
Ok, let's think about this a little more and see what makes sense.
I reverted the changes to the naming of halfAngle, and updated the PR description to reflect the proposed API changes.