ENH: Add Linux ARM in CI and fix char is 'unsigned' bugs by thewtex · Pull Request #5137 · InsightSoftwareConsortium/ITK

thewtex · 2025-01-17T20:00:40Z

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/

Resolves: #5779

Depends on:

Related to upstream:

'char' images written may not be read as signed. BIC-MNI/libminc#133
Fix usage of CHAR_MIN and CHAR_MAX BIC-MNI/libminc#134
Normalize formatting: remove trailing whitespace BIC-MNI/libminc#135 (Style change only)
STYLE: modernize cmake syntax BIC-MNI/libminc#136 (Style change only)

Supercedes:

WIP: [Do not merge] Change the default char type to unsigned #5455

Copilot

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

hjmjohnson

@thewtex The code looks good to me. I rebased it the current upstream/main branch

thewtex · 2026-01-29T03:12:30Z

@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.

hjmjohnson · 2026-02-03T14:24:58Z

Build occurs O.K. but the following tests have failures:

itkMINCImageIOTest-COM-t1_z+_byte_trans-1
itkMINCImageIOTest-COM-t1_z-_byte_trans-1
itkMINCImageIOTest-COM-t1_z+_byte_sag-1 
itkMINCImageIOTest-COM-t1_z-_byte_sag-1 
itkMINCImageIOTest-COM-t1_z+_byte_cor-1 
itkMINCImageIOTest-COM-t1_z-_byte_cor-1 
itkMINCImageIOTest-COM-t1_z+_byte_trans-0
itkMINCImageIOTest-COM-t1_z-_byte_trans-0
itkMINCImageIOTest-COM-t1_z+_byte_sag-0 
itkMINCImageIOTest-COM-t1_z-_byte_sag-0 
itkMINCImageIOTest-COM-t1_z+_byte_cor-0 
itkMINCImageIOTest-COM-t1_z-_byte_cor-0 
itkMINCImageIOTest3     

itkHDF5ImageIOTest

I think the CI component is working as expected, but some code changes are needed to build on Linux ARM environments.

hjmjohnson · 2026-02-03T20:58:07Z

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'

hjmjohnson · 2026-02-07T03:17:08Z

The meta data components of the hd5 file written to disk are different on M3 mac for mac build vs docker linxu-arm build:

h5dump ./Testing/Temporary/ITKIOHDF5/RGBImage.hdf5 ~/PASS_MAC
h5dump ./Testing/Temporary/ITKIOHDF5/RGBImage.hdf5 > ~/FAIL_LINUX
vimdiff ~/PASS_MAC ~/FAIL_LINUX

Modules/Bridge/VtkGlue/src/QuickView.cxx

Modules/Core/Common/include/itkImage.hxx

hjmjohnson · 2026-02-12T13:11:56Z

No description provided.

Copilot

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.

Modules/IO/HDF5/src/itkHDF5ImageIO.cxx

Modules/IO/Meta/test/testMetaImage.cxx

Modules/Core/Common/include/itkConstNeighborhoodIteratorWithOnlyIndex.h

Modules/IO/HDF5/src/itkHDF5ImageIO.cxx

Modules/Core/Common/include/itkMetaDataObject.hxx

Modules/IO/HDF5/src/itkHDF5ImageIO.cxx

hjmjohnson · 2026-02-12T13:29:21Z

@N-Dekker @thewtex @blowekamp @dzenanz

I think this is finally ready for review!

I tested this on Arm-mac with

ITK_FUTURE_REMOVE=ON, compiler flags char->signed
ITK_FUTURE_REMOVE=ON, compiler flags char->unsigned
ITK_FUTURE_REMOVE=OFF, compiler flags char->signed
ITK_FUTURE_REMOVE=OFF, compiler flags char->unsigned
linux-arm docker on arm-mac

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.

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/

Modules/Core/Common/include/itkMetaDataObject.hxx

Modules/Core/Common/test/itkMetaDataDictionaryGTest.cxx

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>.

Add a custom GTest macro for value verification in MetaDataDictionary Introduce `ITK_EXPECT_METADATA_VALUE` macro to enhance readability and consistency in metadata dictionary tests. Refactor metadata test logic to eliminate redundant checks and improve maintainability.

thewtex

🎇 💚 🍏 @hjmjohnson thank you for addressing these remaining tests!!!

dzenanz

Looks good on a glance. Perhaps rename the PR to better reflect everything inside? Or even split into 2 PRs?

github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Enhancement Improvement of existing methods or implementation labels Jan 17, 2025

thewtex requested a review from Copilot January 17, 2025 20:07

Copilot AI reviewed Jan 17, 2025

View reviewed changes

hjmjohnson force-pushed the linux-arm-ci branch from 7bd8b6d to d3ac3f3 Compare January 28, 2026 15:38

hjmjohnson approved these changes Jan 29, 2026

View reviewed changes

hjmjohnson force-pushed the linux-arm-ci branch 3 times, most recently from b9bb4d6 to d77f751 Compare February 2, 2026 13:58

dzenanz requested review from gdevenyi and vfonov February 3, 2026 14:31

hjmjohnson force-pushed the linux-arm-ci branch from f909911 to daab23b Compare February 3, 2026 20:55

github-actions bot added type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module area:IO Issues affecting the IO module labels Feb 3, 2026

hjmjohnson marked this pull request as draft February 3, 2026 20:58

This was referenced Feb 7, 2026

Identify issues when 'char' is used as a numeric type. #5779

Closed

'char' images written may not be read as signed. BIC-MNI/libminc#133

Closed

STYLE: Misc style changes encountered during debugging #5782

Merged

hjmjohnson force-pushed the linux-arm-ci branch from daab23b to 6518bdd Compare February 9, 2026 14:42

github-actions bot added area:Bridge Issues affecting the Bridge module area:Filtering Issues affecting the Filtering module area:Registration Issues affecting the Registration module area:ThirdParty Issues affecting the ThirdParty module labels Feb 9, 2026

hjmjohnson mentioned this pull request Feb 9, 2026

BUG: Removing open HDF5 files causes segmentation fault #5784

Merged

3 tasks

N-Dekker reviewed Feb 10, 2026

View reviewed changes

Modules/Bridge/VtkGlue/src/QuickView.cxx Outdated Show resolved Hide resolved

N-Dekker reviewed Feb 10, 2026

View reviewed changes

Modules/Core/Common/include/itkImage.hxx Outdated Show resolved Hide resolved

hjmjohnson force-pushed the linux-arm-ci branch from 6518bdd to 64ec0c7 Compare February 12, 2026 03:52

github-actions bot removed the area:ThirdParty Issues affecting the ThirdParty module label Feb 12, 2026

hjmjohnson mentioned this pull request Feb 12, 2026

WIP: [Do not merge] Change the default char type to unsigned #5455

Closed

hjmjohnson requested review from N-Dekker, blowekamp, Copilot and hjmjohnson February 12, 2026 13:12

Copilot started reviewing on behalf of hjmjohnson February 12, 2026 13:13 View session

hjmjohnson marked this pull request as ready for review February 12, 2026 13:16

Copilot AI reviewed Feb 12, 2026

View reviewed changes

hjmjohnson force-pushed the linux-arm-ci branch from 14a3198 to 8269ff4 Compare February 12, 2026 14:31

ENH: Test Linux ARM in CI

6aff639

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/

hjmjohnson force-pushed the linux-arm-ci branch from 8269ff4 to 21163c8 Compare February 12, 2026 14:34

github-actions bot removed area:Bridge Issues affecting the Bridge module area:Filtering Issues affecting the Filtering module area:Registration Issues affecting the Registration module labels Feb 12, 2026

blowekamp reviewed Feb 12, 2026

View reviewed changes

Modules/Core/Common/include/itkMetaDataObject.hxx Outdated Show resolved Hide resolved

blowekamp reviewed Feb 12, 2026

View reviewed changes

Modules/Core/Common/test/itkMetaDataDictionaryGTest.cxx Show resolved Hide resolved

hjmjohnson added 2 commits February 12, 2026 12:10

hjmjohnson force-pushed the linux-arm-ci branch from 21163c8 to 036327a Compare February 12, 2026 18:27

blowekamp self-requested a review February 12, 2026 18:45

thewtex commented Feb 12, 2026

View reviewed changes

hjmjohnson requested a review from dzenanz February 12, 2026 20:34

dzenanz approved these changes Feb 12, 2026

View reviewed changes

hjmjohnson changed the title ~~ENH: Test Linux ARM in CI~~ ENH: Add Linux ARM in CI and fix char is 'unsigned' bugs Feb 12, 2026

hjmjohnson merged commit e64fa64 into InsightSoftwareConsortium:main Feb 12, 2026
18 checks passed

Uh oh!

Conversation

thewtex commented Jan 17, 2025 • edited by hjmjohnson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

hjmjohnson left a comment

Choose a reason for hiding this comment

Uh oh!

thewtex commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hjmjohnson commented Feb 3, 2026

Uh oh!

hjmjohnson commented Feb 3, 2026

Uh oh!

hjmjohnson commented Feb 7, 2026

Uh oh!

Uh oh!

Uh oh!

hjmjohnson commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hjmjohnson commented Feb 12, 2026

Uh oh!

Uh oh!

Uh oh!

thewtex left a comment

Choose a reason for hiding this comment

Uh oh!

dzenanz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

thewtex commented Jan 17, 2025 •

edited by hjmjohnson

Loading

thewtex commented Jan 29, 2026 •

edited

Loading

hjmjohnson commented Feb 12, 2026 •

edited

Loading