Getting different results from two identical matrices

[n8xm]

If I run the following code:

from sophus import SE3
import numpy as np
import math

angle = math.pi/4;
T1 = SE3.rotX(angle)
T1_numpy = T1.matrix();
T2_numpy = np.array([[1, 0, 0, 0],
                     [0, math.cos(angle), -math.sin(angle), 0],
                     [0, math.sin(angle), math.cos(angle), 0],
                     [0, 0, 0, 1]])

print(np.allclose(T1_numpy,T2_numpy)) #result is "true"
print(SE3(T1_numpy))
print(SE3(T2_numpy))

It is evident that T1_numpy and T2_numpy contain the same values up to some (very small) epsilon, because print(np.allclose(T1_numpy,T2_numpy)) returns true.

However, print(SE3(T1_numpy)) gives me the following output:

[[ 1.          0.          0.          0.        ]
 [ 0.          0.70710678 -0.70710678  0.        ]
 [ 0.          0.70710678  0.70710678  0.        ]
 [ 0.          0.          0.          1.        ]]

Whereas print(SE3(T2_numpy)) gives me this:

[[ 1.         -0.          0.          0.        ]
 [ 0.          0.70710678  0.70710678  0.        ]
 [-0.         -0.70710678  0.70710678  0.        ]
 [ 0.          0.          0.          1.        ]]

The two results should be the same! Instead, one is the inverse of the other! What is going on here? Is this a bug? Or have I missed something really obvious?

[n8xm]

T2_numpy turns out to be "C contiguous" whereas T1_numpy is "F contiguous". (See the numpy documentation, specifically the C_CONTIGUOUS and F_CONTIGUOUS flags for more info.) But these bindings should account for this, no?

Update: this appears to be a subtle issue with Eigency. From their README, under the "Storage layout" header, it seems that "to deal with the problem either in Python", we must:

use order="F" when constructing your numpy array

My assessment is that PySophus should change order to F before passing to Eigency.

[arntanguy]

Thanks for pointing this out!

I have checked your example, and it did indeed result in erroneous results. I implemented the conversion from C_CONTIGUOUS to F_CONTIGUOUS as suggested, which unfortunately adds a copy operation.

I also added a basic unit test based on your example to test the conversion.

Could you confirm that this commit fully solves your issue?