itkMatrix in Python Wrapping fails to instantiate for certain combinations

[PranjalSahu]

Instantiating itkMatrix as the following fails for certain combinations of dimensions:

a = itk.Matrix[itk.F, 2, 2]()
a = itk.Matrix[itk.F, 4, 4]()

Object creation fails with the following error message:

vnl_copy = type(vnl_reference)(vnl_reference)
TypeError: cannot create 'SwigPyObject' instances

4x4 is needed for the MetaData object. Refer https://github.com/InsightSoftwareConsortium/ITK/issues/3520

It works for all combinations for double (i.e. itk.D) but fails for certain combinations of itk.F. Looks like changes introduced in PR https://github.com/InsightSoftwareConsortium/ITK/pull/3305 introduced this bug. I am running a build just before this commit to confirm.

cc @N-Dekker

Expected behavior

>>> a = itk.Matrix[itk.F, 3, 3]()
>>> print(a)
itkMatrixF33 ([[0.0, 0.0, 0.0], [0.0, 0.0, 0.0], [0.0, 0.0, 0.0]])

Actual behavior

Reproducibility

100%

Versions

Since 5.3rc4.

Works fine for version itk 5.2.1.post1.

Environment

Additional Information

[N-Dekker]

@PranjalSahu Thanks for the cc! Are you sure that this issue is caused by the Matrix(const TElement (&)[VRows][VColumns]) constructor introduced with pull request #3305 commit 7dd9e49534f9a9175cfba7f1105a00554efd2ed6 ? Specifically, this one: https://github.com/InsightSoftwareConsortium/ITK/blob/9ed9070c74ae00cb7c258bc550cf2dd6ae469c43/Modules/Core/Common/include/itkMatrix.h#L232-L233

If that one really causes the issue, could you possibly figure out if SWIG could be instructed somehow to ignore this constructor? Possibly using SWIG %ignore? https://www.swig.org/Doc1.3/SWIG.html#SWIG_rename_ignore (I'm just guessing, I just don't know.)

It would be nice if SWIG %ignore would do the job, and if this specific constructor could then be non-templated instead. As in:

  // SWIG, please ignore this constructor!
  explicit Matrix(const T (&elements)[VRows][VColumns])
    : m_Matrix(&elements[0][0])
  {}

[PranjalSahu]

@N-Dekker Actually this commit is breaking it

ENH: Make itk::Matrix trivially copyable, following Rule of Zero

[PranjalSahu]

I am adding some float combinations in the GTest to see the behavior. Because the python wrappings are failing only for float combinations.

[N-Dekker]

Actually this commit is breaking it

ENH: Make itk::Matrix trivially copyable, following Rule of Zero

Thanks! Actually that commit has quite a few code changes. Do you know which particular code change caused this issue? For example, would it be sufficient to only just restore the original default-constructor of itk::Matrix?

  /** Default constructor. */
  Matrix()
    : m_Matrix(NumericTraits<T>::ZeroValue())
  {}

Instead of the new Matrix() = default ?

As a side note, I believe Python wrapping support for 64-bit double is often more useful than support for 32-bit float, because the built-in floating point type of Python itself is 64-bit.

[PranjalSahu]

@N-Dekker I am trying your suggested changes.

[PranjalSahu]

Simply reverting that commit didn't help. But I found something that might give us come clue.

The object type is different for MatrixF33 and (MatrixF44, matrixF22 etc.). Only MatrixF33 is able to instantiate for float. Other combinations are failing.

For MatrixF33 (This works) Printing the type(vnl_reference) and vnl_reference = self.__GetVnlMatrix_orig__()

<itk.vnl_matrix_fixedPython.vnl_matrix_fixedF_3_3; proxy of <Swig Object of type 'vnl_matrix_fixedF_3_3 *' at 0x7f0ed8dc72a0> >
<class 'itk.vnl_matrix_fixedPython.vnl_matrix_fixedF_3_3'>
itkMatrixF33 ([[0.0, 0.0, 0.0], [0.0, 0.0, 0.0], [0.0, 0.0, 0.0]])

For MatrixF32 (And this does not)

<Swig Object of type 'vnl_matrix_fixed< float,3,2 > *' at 0x7f7d1a0632a0>
<class 'SwigPyObject'>

[PranjalSahu]

I am doing a binary search to see where it fails. I saw that it worked for this commit N-Dekker/Work-around-GCC4-error-is_trivially_copyable after reverting ENH: Make itk::Matrix trivially copyable, following Rule of Zero

Proceeding further.

[PranjalSahu]

Ok, I got the issue. vnl_matrix_fixed is not wrapped for float combinations ! Checking the build now.

vnl_matrix_fixed.wrap

Update: It works !

[PranjalSahu]

Solution PR https://github.com/InsightSoftwareConsortium/ITK/pull/3553