pyxem / orix

Analysing crystal orientations and symmetry in Python
https://orix.readthedocs.io
GNU General Public License v3.0
78 stars 45 forks source link

`ax2qu` does not raise anymore if last dimension is `1` #493

Closed harripj closed 2 months ago

harripj commented 2 months ago

Description of the change

ax2qu conversion function throws if the arrays are multidimensional and last axis size is 1, even though they are broadcastable.

I appreciate having a redundant axis in an array is not ideal, but sometimes it is easier to work with it to keep the input and output shapes consistent 😄.

I have looked into the code and there appears to be a lot of shape juggling because the internal functions are numba jitted and ultimately require 2d arrays. These lines in particluar seem to be the issue: https://github.com/pyxem/orix/blob/856491e341efc695da7b483fd108491a425f884f/orix/quaternion/_conversions.py#L557-L561

I believe we should also check here if axes.shape[:-1] == angles.shape and this will fix the issue.

Progress of the PR

Minimal example of the bug fix or new feature

>>> from orix.quaternion import Quaternion
>>> import numpy as np

>>> axes = np.random.randn(2, 1, 3)
>>> angles = np.random.randn(*axes.shape[:-1])

>>> Quaternion.from_axes_angles(axes, angles)
ValueError: The dimensions of axes (2, 1) and angles (2,) are incompatible. The dimensions must match or one must be a singular value.

With the fix in this PR:

>>> Quaternion.from_axes_angles(axes, angles)
Quaternion (2, 1)
[[[ 0.9999 -0.0058 -0.0025 -0.0078]]

 [[ 0.9386 -0.1434 -0.1071  0.295 ]]]

For reviewers

pc494 commented 2 months ago

This looks good to me, will leave it to @hakonanes to review and merge though.

harripj commented 2 months ago

@hakonanes thanks for reviewing! Yes it would be great to include this in the patch release. I've rebased this PR.