Native quaternion implementation #166
Conversation
arahlin
force-pushed
the
g3quat
branch
4 times, most recently
from
511431b
to
5c1907f
Compare
This PR removes the dependency on boost::math::quaternion in favor of a minimal native implementation, which includes simple arithmetic and comparison operators, and conj, abs, norm and pow function implementations. The class also provides versor() and is_versor() methods for creating and testing for unit quaternions, used in many pointing operations in the maps package. The G3Quat object is a true serializeable G3FrameObject with a numpy buffer interface. The derived G3VectorQuat and G3MapQuat classes maintain backward compatibility for deserializing data stored on disk.
|
I had one large-scale question. Making G3Quat a frame object instead of a primitive type makes a few things more complicated:
Have you looked at the change in storage size or other performance metrics? |
|
I haven't yet. That said, it would be straightforward to walk back the frameobject change and leave the serialization and vectorization as is. |
|
I think you're right that the G3VectorQuat object ends up 50% larger, just based on the change in the length of |
Use a separate G3Quat derived class for serialization into a frame, analogously to the simple G3Data quantities.
| @@ -1536,7 +1536,7 @@ PYBINDINGS("maps") { | |||
| "a disc of the given radius at the given sky coordinates.") | |||
|
|
|||
| .def("query_disc", | |||
| (std::vector<size_t> (G3SkyMap::*)(quat, double) const) | |||
| (std::vector<size_t> (G3SkyMap::*)(const Quat &, double) const) | |||
There was a problem hiding this comment.
Is there any reason to do this? If quat is the same type as Quat basically, I'm not sure this is any more efficient.
There was a problem hiding this comment.
Fair, this made some more sense when Quat was derived from G3FrameObject.
There was a problem hiding this comment.
It looks like it's O(10ns) of overhead for the value argument vs const reference for a single operation. Pretty small, but might add up?
|
This basically looks good to me. It looks like it would reduce the diff by quite a lot if you did s/Quat/quat. Is there a reason to change the class name? |
Just to differentiate this class from the boost one, and to make it clear that it's a class object with |
This PR removes the dependency on boost::math::quaternion in favor of a minimal native implementation, which includes simple arithmetic and comparison operators, and conj, abs, norm and pow function implementations. The Quat object is serializeable and provides a numpy buffer interface.
Closes #143.