ENH: Test Linux ARM in CI#5137
ENH: Test Linux ARM in CI#5137thewtex wants to merge 2 commits intoInsightSoftwareConsortium:mainfrom
Conversation
7bd8b6d to
d3ac3f3
Compare
hjmjohnson
left a comment
There was a problem hiding this comment.
@thewtex The code looks good to me. I rebased it the current upstream/main branch
|
@hjmjohnson thanks, but the workflow is invalid: https://github.com/InsightSoftwareConsortium/ITK/actions/runs/21444681186/workflow And there are also test failures and warnings that need to be addressed. |
b9bb4d6 to
d77f751
Compare
|
Build occurs O.K. but the following tests have failures: I think the CI component is working as expected, but some code changes are needed to build on Linux ARM environments. |
f909911 to
daab23b
Compare
|
FYI: I think the crux of the problem lies in how MetaData is handled in ITK for HDF5 image IO. Failure ExposeMetaData 'TestBool'
Failure ExposeMetaData 'TestUChar'
Incorrect meta value read in for TestUChar '
' != 'u' |
daab23b to
6518bdd
Compare
6518bdd to
64ec0c7
Compare
|
No description provided. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@N-Dekker @thewtex @blowekamp @dzenanz I think this is finally ready for review! I tested this on Arm-mac with
I used h5dump on the created .hdf files to verify that the metadata data types were preserved in all but one case. PREVIOUSLY : If an .h5 image file was written with a metadata element of type 'char', it was written in the .h5 file as either explicitly unsigned or signed (see image in previous comment). If it were attempted to be read by a binary with a different signedness of 'char', it would not match the requested type and would fail to read the meta-data. The current code retains backward compatibility if old h5 files with 'char' based meta data as long as the only ASCII characters (i.e. values between 0-127) were used (all the test cases), OR if the the files were written from platforms where the 'char' is a signed data type. In other words, only files that were written from a platform where 'char' was unsigned AND contained values larger than 127 will be interpreted differently, but that is impossible to determine if from the file. |
14a3198 to
8269ff4
Compare
GitHub Actions Linux ARM runners are now available for free for open source projects. https://github.blog/changelog/2025-01-16-linux-arm64-hosted-runners-now-available-for-free-in-public-repositories-public-preview/
Initial resolution for addressing ambiguitiy in 'char' signed behavior desicribed in issue Related to: InsightSoftwareConsortium#5779 `VerifyMetaData` function to eliminate redundant metadata handling code in `HDF5ImageIOTest` and `MetaDataDictionaryGTest`. This simplifies test logic and improves maintainability. Reduce duplication of common testing. Provide more failure diagnostics to assist with pin-pointing the failure locations. Replace `dynamic_cast` with `static_cast` and explicit type information checks for clearer and more efficient metadata handling. On linux-arm <char> and <unsigned char> are the same types. On most other platforms, <char> is the same as <signed char>.
8269ff4 to
21163c8
Compare
| /* Used only to avoid duplicate code in itkMetaDictionaryGTest.cxx and itkHDF5ImageIOTest.cxx */ | ||
| template <typename T> | ||
| int | ||
| VerifyMetaDataPrivateTestingUtility(const itk::MetaDataDictionary & metaDict, |
There was a problem hiding this comment.
This function should go in the TestKernel module if it's used for testing in multiple modules.
|
|
||
| template <typename T> | ||
| static int | ||
| CheckMetaData(itk::MetaDataDictionary & metaDict, const std::string & key, const T & knownValue) |
There was a problem hiding this comment.
Consider adding as a GTestPredicate in TestKernel/../itkGTestPredicate.h


GitHub Actions Linux ARM runners are now available for free for open
source projects.
https://github.blog/changelog/2025-01-16-linux-arm64-hosted-runners-now-available-for-free-in-public-repositories-public-preview/
Depends on:
Related to upstream:
Supercedes:
chartype to unsigned #5455