InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.4k stars 664 forks source link

Serialization Test Failures in Windows CI #3310

Closed tbirdso closed 2 years ago

tbirdso commented 2 years ago

Description

CDash results show failures in Mesh and Point Set serialization Python tests.

Steps to Reproduce

ctest -C Release -R MeshSerialization

Expected behavior

Test passes

Actual behavior

Traceback (most recent call last):
  File "D:\a\1\s-build\Wrapping\Generators\Python\itk\support\template_class.py", line 525, in __getitem__
    this_item = self.__template__[key]
KeyError: (<itkCType unsigned long>, <class 'itk.itkPointPython.itkPointF3'>)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "D:/a/1/s/Modules/Core/Mesh/wrapping/test/itkMeshSerializationTest.py", line 36, in <module>
    v_point = itk.VectorContainer[itk.UL, PointType].New()
  File "D:\a\1\s-build\Wrapping\Generators\Python\itk\support\template_class.py", line 529, in __getitem__
    raise itk.TemplateTypeError(self, key)
itk.support.extras.TemplateTypeError: itk.VectorContainer is not wrapped for input type `itk.UL, itk.Point[itk.F,3]`.

Reproducibility

100%

Versions

main branch

Environment

Windows 10, Python 3.8.5

Additional Information

Issue stems from inconsistent IdentifierType wrappings across platforms where ITKM_IT may be replaced with itk.UL or itk.ULL depending on architecture. This leads to difficulty in designing tests for objects such as VectorContainer that require an IdentifierType key in their template.

tbirdso commented 2 years ago

I missed the open PR, thanks @dzenanz and @PranjalSahu !

tbirdso commented 2 years ago

@PranjalSahu You may have already seen this, it looks like just MeshSerializationTest is failing now with a new error:

Traceback (most recent call last):
  File "D:/a/1/s/Modules/Core/Mesh/wrapping/test/itkMeshSerializationTest.py", line 66, in <module>
    mesh.SetCellsArray(cells_vector, itk.CommonEnums.CellGeometry_TRIANGLE_CELL)
TypeError: in method 'itkMeshD3_SetCellsArray', argument 2 of type 'itkVectorContainerULLULL *'
Additional information:
Wrong number or type of arguments for overloaded function 'itkMeshD3_SetCellsArray'.
  Possible C/C++ prototypes are:
    itkMeshD3::SetCellsArray(itkVectorContainerULLULL *)
    itkMeshD3::SetCellsArray(itkVectorContainerULLULL *,int)

itkTestDriver: Process exited with return value: 1

CDash results

PranjalSahu commented 2 years ago

There seems to be a difference for enums in windows and linux. I will check it.

tbirdso commented 2 years ago

Thanks @PranjalSahu.

Probably unrelated to enums, but as a note on the previous resolution for posterity, a few ITK Python constructors have slightly different wrappings on Windows vs Linux due to different IdentifierType specifiers for unsigned long vs unsigned long long system architecture. IdentityType manglings are described in the ITK Software Guide on page 223 under the wrapping table. A good way to approach this is what @PranjalSahu did in #3309 with an inline platform check during the test.

I'm not sure that itk.UL vs itk.ULL will account for differences in enum behavior, though.

PranjalSahu commented 2 years ago

Yes, that was my first impression due to the message related to 2nd argument in the SetCellsArray. I looked closely at the code and it probably originates due to this line cells_array = np.zeros([NumberOfCells, Dimension], np.uint)

I will run the code end-to-end on the windows platform this time.

tbirdso commented 2 years ago

@PranjalSahu Agreed. I would recommend taking a look at _get_itk_pixelid in extras.py as well to verify that the numpy <-> ITK type conversion is happening as expected, possibly tracing through with pdb.set_trace().

PranjalSahu commented 2 years ago

Will raise a PR soon. We need to add a conditional entry for np.uint64 in _get_itk_pixelid. Also in my test, I have to use np.uint64.

PranjalSahu commented 2 years ago

ctest tip is great! It will help me in testing code faster in the future.