Currently, Resource.rotate reads
def rotate(self, x: float = 0, y: float = 0, z: float = 0):
"""Rotate counter-clockwise by the given number of degrees."""
self.rotation.x = (self.rotation.x + x) % 360
self.rotation.y = (self.rotation.y + y) % 360
self.rotation.z = (self.rotation.z + z) % 360
# Rotation is part of the resource's state; notify subscribers (e.g. the
# Visualizer) so they can re-render.
self._state_updated()However, rotation represents rotations as Euler angles (as can be inferred from Rotation.get_rotation_matrix). The above implementation siltently gives wrong results. A trivial example is rotating a resource which currently is rotated 90 degrees around z about the x axis. Because Euler angles in the roll-pitch-yaw convention rotate about x first, simply adding to the roll value affectively acts as a rotation around y instead of the requested x.
The correct composition behaviour of Euler angles unfortunately is very nontrivial; requiring forward and inverse trigonometrics. That also leads to certain rotations have an ill-defined Euler angle representation ("gimbal lockup") where the inverse trigonometrics diverge. The standard solution in robotics is to use quaternions to represent rotations, where composition is compartively simple, and are free of divergences. Thanks to the Rotation abstraction in PLR, such a refactor could be fairly local. The biggest issue are the places where the abstractions are leaked, and code directly looks into Rotation. The one place I have in mind (LiquidHandler.drop_resource) also silently ignores all but yaw, so that one benefits from a refactor anyway (should probably raise ValueError on significantly off-axis rotations).
Currently,
Resource.rotatereadsHowever,
rotationrepresents rotations as Euler angles (as can be inferred fromRotation.get_rotation_matrix). The above implementation siltently gives wrong results. A trivial example is rotating a resource which currently is rotated 90 degrees around z about the x axis. Because Euler angles in the roll-pitch-yaw convention rotate about x first, simply adding to the roll value affectively acts as a rotation around y instead of the requested x.The correct composition behaviour of Euler angles unfortunately is very nontrivial; requiring forward and inverse trigonometrics. That also leads to certain rotations have an ill-defined Euler angle representation ("gimbal lockup") where the inverse trigonometrics diverge. The standard solution in robotics is to use quaternions to represent rotations, where composition is compartively simple, and are free of divergences. Thanks to the
Rotationabstraction in PLR, such a refactor could be fairly local. The biggest issue are the places where the abstractions are leaked, and code directly looks intoRotation. The one place I have in mind (LiquidHandler.drop_resource) also silently ignores all but yaw, so that one benefits from a refactor anyway (should probably raiseValueErroron significantly off-axis rotations).