Fix convention of PolynomialTensor basis change#805
Conversation
In general, we rotate a matrix M to M' is to apply a rotation matrix R:
M' = R @ M @ R.T
The corresponding Einstein notation is
M'^{p_1p_2} = R^{p_1}_{a_1} R^{p_2}_{b_1} M^{a_1a_2}
|
@arettig Would you be able to take a look at this and check the technical correctness? (No rush.) |
arettig
left a comment
arettig
left a comment
There was a problem hiding this comment.
This is a good catch. The PolynomialTensor rotation code currently does not match the expression in the documentation. We should change this to match standard convention (and our own documentation), but this is a potentially breaking change for anyone using this function currently.
It would also be a good idea to change the 90 degree rotation test added in this PR to some different rotation as this test actually passes with the current (incorrect) implementation.
| polynomial_tensor.rotate_basis(rotation_matrix_reverse) | ||
| self.assertEqual(polynomial_tensor, want_polynomial_tensor) | ||
|
|
||
| def test_rotate_basis_90deg(self): |
There was a problem hiding this comment.
This test actually does not catch the bug you pointed out since for this specific case
4 participants
In general, we rotate a matrix M to M' is to apply a rotation matrix R: M' = R @ M @ R.T
The corresponding Einstein notation is
M'^{p_1p_2} = R^{p_1}{a_1} R^{p_2}{b_1} M^{a_1a_2}