Describe the bug
Hello,
The function color_orientations() under material/unitcell behaves differently depending on whether a single rotation matrix (3x3) or a stack of rotation matrices (Nx3x3) is passed in. I don't think this is the intended behavior.
In the case of a single rotation matrix (currently lines 1390-1391), the if statement calls np.atleast_3d(rmats).T, which reshapes the array to 1x3x3 but also transposes the rotation matrix. For example, if I generate the following matrix:
np.arange(9).reshape(3, 3)
Out:
array([[0, 1, 2],
[3, 4, 5],
[6, 7, 8]])
and apply this function, the following is returned:
np.atleast_3d(np.arange(9).reshape(3, 3)).T
Out:
array([[[0, 3, 6],
[1, 4, 7],
[2, 5, 8]]])
If a stack of rotation matrices is passed in (else case, currently lines 1392-1396), then no transpose is applied to the matrices. As such, different colors will be returned depending on whether the rotation matrices are passed in individually or all at once. As currently written, input rotation matrices would need to be crystal-to-sample for the stack (since these only get transposed once in the for loop in lines 1403-1404) versus sample-to-crystal for individual matrices (since these get transposed twice) to obtain the intended result.
np.atleast_3d(rmats.T).T or rmats[None] are examples of potential replacements for line 1391 that would unify the behavior of the function by reshaping without transposing:
np.atleast_3d(np.arange(9).reshape(3, 3).T).T
Out:
array([[[0, 1, 2],
[3, 4, 5],
[6, 7, 8]]])
np.arange(9).reshape(3, 3)[None]
Out[70]:
array([[[0, 1, 2],
[3, 4, 5],
[6, 7, 8]]])
Impact
Will eventually cause someone a problem
Reproduction
Below is an array with two example rotation matrices:
rots = np.array([[[ 0.8513851 , 0.51820825, -0.08126269],
[-0.51602112, 0.85525329, 0.04758169],
[ 0.0941574 , 0.00142293, 0.99555631]],
[[ 0.93586175, -0.34601698, -0.06659611],
[ 0.3489187 , 0.93637851, 0.03809232],
[ 0.04917857, -0.05888577, 0.99705262]]])
If I pass these rotation matrices in individually, the following outputs are obtained for the material I'm using (rounded to 3 decimal places):
np.round(mat.unitcell.color_orientations(rots[0]), 3)
Out: array([[1. , 0. , 0.085]])
np.round(mat.unitcell.color_orientations(rots[1]), 3)
Out: array([[1. , 0. , 0.069]])
If I pass the stack of rotation matrices in, different results are obtained:
np.round(mat.unitcell.color_orientations(rots), 3)
Out:
array([[1. , 0.086, 0.001],
[0.995, 0.068, 0.001]])
These results can be reproduced by passing in the individual matrices with a transpose:
np.round(mat.unitcell.color_orientations(rots[0].T), 3)
Out: array([[1. , 0.086, 0.001]])
np.round(mat.unitcell.color_orientations(rots[1].T), 3)
Out: array([[0.995, 0.068, 0.001]])
Version
This is in the current release, and it looks like it's been this way for ~5 years or so.
Affected Workflows
Describe the bug
Hello,
The function color_orientations() under material/unitcell behaves differently depending on whether a single rotation matrix (3x3) or a stack of rotation matrices (Nx3x3) is passed in. I don't think this is the intended behavior.
In the case of a single rotation matrix (currently lines 1390-1391), the if statement calls
np.atleast_3d(rmats).T, which reshapes the array to 1x3x3 but also transposes the rotation matrix. For example, if I generate the following matrix:and apply this function, the following is returned:
If a stack of rotation matrices is passed in (else case, currently lines 1392-1396), then no transpose is applied to the matrices. As such, different colors will be returned depending on whether the rotation matrices are passed in individually or all at once. As currently written, input rotation matrices would need to be crystal-to-sample for the stack (since these only get transposed once in the for loop in lines 1403-1404) versus sample-to-crystal for individual matrices (since these get transposed twice) to obtain the intended result.
np.atleast_3d(rmats.T).Torrmats[None]are examples of potential replacements for line 1391 that would unify the behavior of the function by reshaping without transposing:Impact
Will eventually cause someone a problem
Reproduction
Below is an array with two example rotation matrices:
If I pass these rotation matrices in individually, the following outputs are obtained for the material I'm using (rounded to 3 decimal places):
If I pass the stack of rotation matrices in, different results are obtained:
These results can be reproduced by passing in the individual matrices with a transpose:
Version
Affected Workflows