Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

COMP: Consolidate Linux ARM build script #236

Merged
merged 4 commits into from
Jan 2, 2023
Merged

Conversation

tbirdso
Copy link
Contributor

@tbirdso tbirdso commented Dec 1, 2022

Several quality-of-life fixes and updates for ARM support:

  • Consolidates ARM module script with x86_64 module script for easier
    maintenance and comparison between procedures. Internal ARM script is
    preserved as a wrapper around generic internal build script.
  • Splits MANYLINUX_VERSION variable into MANYLINUX_VERSION prefix
    and TARGET_ARCH postfix to better reflect available images.
  • Uses sudo to avoid permissions failure when running docker or
    cleaning up files on ARM systems

Applies changes introduced by @thewtex in #234.

Tests have passed at https://github.com/InsightSoftwareConsortium/ITKSplitComponents/actions/runs/3736106376/jobs/6340059855. Note that ARM builds on Github Actions runners are significantly slower than x86/x64 builds due to emulation. ITKSplitComponents Linux jobs previously completed in ~10 minutes and now complete in ~30 minutes with the addition of ARM wheels.

co-authored-by: Matt McCormick matt.mccormick@kitware.com

@tbirdso
Copy link
Contributor Author

tbirdso commented Dec 1, 2022

@tbirdso tbirdso force-pushed the amend-arm-builds branch 2 times, most recently from da03573 to 2b7a48c Compare December 2, 2022 15:52
@tbirdso
Copy link
Contributor Author

tbirdso commented Dec 2, 2022

After navigating several incremental issues applying build scripts for ARM module builds I've encountered a wrapping compilation issue:

FAILED: /work/ITK-cp38-cp38-manylinux_2_28_aarch64/Wrapping/castxml_inputs/itkSplitComponentsImageFilter.xml 
cd /work/_skbuild/linux-aarch64-3.8/cmake-build/Wrapping/Modules/SplitComponents && /work/_skbuild/linux-aarch64-3.8/cmake-build/Wrapping/Generators/CastXML/castxml/bin/castxml -o /work/ITK-cp38-cp38-manylinux_2_28_aarch64/Wrapping/castxml_inputs/itkSplitComponentsImageFilter.xml --castxml-gccxml --castxml-start _wrapping_ --castxml-cc-gnu "(" /opt/rh/gcc-toolset-12/root/usr/bin/c++ -std=c++14 ")" -w -c @/work/ITK-cp38-cp38-manylinux_2_28_aarch64/Wrapping/castxml_inputs/.castxml.inc /work/ITK-cp38-cp38-manylinux_2_28_aarch64/Wrapping/castxml_inputs/itkSplitComponentsImageFilter.cxx
...
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/aarch64-redhat-linux/12/../../../../include/c++/12/bits/new_allocator.h:158:2: error: call to '__builtin_operator_delete' selects non-usual deallocation function
        _GLIBCXX_OPERATOR_DELETE(_GLIBCXX_SIZED_DEALLOC(__p, __n));
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Failed build with logs is at https://github.com/tbirdso/ITKSplitComponents/actions/runs/3604194121/jobs/6073281299

As this appears to occur in the first applied CastXML step I will open an issue on that repo for help.

@thewtex
Copy link
Member

thewtex commented Dec 5, 2022

@tbirdso I am wondering if this is an incompatibility with the ITKPythonBuilds tarball (built with the gcc-toolset-11) and the gcc-toolset-12 in the manylinux aarch64 latest image?

I recommend rebasing on master to integrate #237. Then rebuild the tarball on blaster. This will take a while. Then test the module wheel script locally with the built tarball.

Since we will need updated itk-* aarch64 wheels, I suggest we defer this update to 5.3.1.

@tbirdso
Copy link
Contributor Author

tbirdso commented Dec 6, 2022

I am wondering if this is an incompatibility with the ITKPythonBuilds tarball (built with the gcc-toolset-11) and the gcc-toolset-12 in the manylinux aarch64 latest image?

Thanks @thewtex , I think this is spot on. I have consolidated discussion in InsightSoftwareConsortium/ITK#3785.

@tbirdso tbirdso force-pushed the amend-arm-builds branch 3 times, most recently from 17ec3c6 to 4e58e1f Compare December 19, 2022 22:10
tbirdso added a commit to InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction that referenced this pull request Dec 19, 2022
Exposes input parameter to allow the user to build for multiple Linux
toolsets and target architectures via available manylinux docker images.

Depends on ARM support introduced in ITKPythonPackage:
InsightSoftwareConsortium/ITKPythonPackage#236
tbirdso added a commit to InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction that referenced this pull request Dec 19, 2022
Exposes input parameter to allow the user to build for multiple Linux
toolsets and target architectures via available manylinux docker images.

Depends on ARM support introduced in ITKPythonPackage:
InsightSoftwareConsortium/ITKPythonPackage#236
@tbirdso
Copy link
Contributor Author

tbirdso commented Dec 19, 2022

Re-running testing at InsightSoftwareConsortium/ITKSplitComponents#67

@tbirdso tbirdso requested a review from thewtex December 20, 2022 01:25
@tbirdso tbirdso marked this pull request as ready for review December 20, 2022 01:25
@tbirdso
Copy link
Contributor Author

tbirdso commented Dec 20, 2022

Ready for review as CI jobs are passing. However, it should be noted that Python packaging jobs build but do not load and test wheels. Once ARM Linux wheel artifacts are available from the ITKSplitComponents test run it would be good to verify that those can be used on an ARM platform. It may take me several days to get back to testing those, or someone else can test to verify.

Steps to test:

  1. Pull ITKSplitComponents Linux ARM wheel artifacts from https://github.com/tbirdso/ITKSplitComponents/actions/runs/3736220631
  2. Obtain access to a Linux ARM machine, such as running an ARM instance on AWS EC2
  3. Install the corresponding Python wheel
  4. Install ITK with pip install itk==v5.3.0
  5. Put the ITKSplitComponents wheel on the machine and install the local wheel in the Python environment
  6. In a Python shell try loading a filter from the module: import itk; itk.SplitComponentsImageFilter.GetTypes()
  7. (Optional) Try reproducing example functionality from the ITKSplitComponents example notebook

EDIT: Python load test failed; see comments below.

tbirdso added a commit to InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction that referenced this pull request Dec 20, 2022
Exposes input parameter to allow the user to build for multiple Linux
toolsets and target architectures via available manylinux docker images.

Depends on ARM support introduced in ITKPythonPackage:
InsightSoftwareConsortium/ITKPythonPackage#236
@tbirdso
Copy link
Contributor Author

tbirdso commented Dec 20, 2022

EDIT: On review it looks like there was an issue in Python 3.11 packages resulting in only ARM wheel artifacts being made available from the run, will need to investigate.

Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tbirdso
Copy link
Contributor Author

tbirdso commented Dec 22, 2022

Getting back to the 3.11 log, it looks like the issue may have simply been that I force pushed ITKPythonPackage updates too quickly.

Updating build scripts to InsightSoftwareConsortium/ITKPythonPackage@d45219286484dafb746591e3801f4e860dfd29ec
Cloning into 'IPP-tmp'...
~/work/ITKSplitComponents/ITKSplitComponents/IPP-tmp ~/work/ITKSplitComponents/ITKSplitComponents
fatal: reference is not a tree: d45219286484dafb746591e3801f4e860dfd29ec

Will re-run testing in ITKSplitComponents.

EDIT: Testing at https://github.com/InsightSoftwareConsortium/ITKSplitComponents/actions/runs/3759019847/jobs/6388068372

EDIT: Build succeeded on retry.

@tbirdso
Copy link
Contributor Author

tbirdso commented Dec 23, 2022

When attempting to install locally, load, and test on an ARM Linux instance I am seeing the following failure:

>>> itk.SplitComponentsImageFilter.GetTypes()
Loading ITKPyBase... done
Loading ITKCommon... done
Loading SplitComponents... Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ubuntu/.local/lib/python3.8/site-packages/itk/support/lazy.py", line 138, in __getattribute__
    base.itk_load_swig_module(module, namespace)
  File "/home/ubuntu/.local/lib/python3.8/site-packages/itk/support/base.py", line 132, in itk_load_swig_module
    l_module = loader.load(swig_module_name)
  File "/home/ubuntu/.local/lib/python3.8/site-packages/itk/support/base.py", line 291, in load
    l_spec.loader.exec_module(l_module)  # pytype: disable=attribute-error
  File "<frozen importlib._bootstrap_external>", line 848, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/ubuntu/.local/lib/python3.8/site-packages/itk/support/../SplitComponentsPython.py", line 13, in <module>
    from . import _SplitComponentsPython
ImportError: libtbb.so.12: cannot open shared object file: No such file or directory

It looks like the TBB library is not being packaged with the ITKSplitComponents wheel as expected. I have compared the contents of the 2_28_aarch64 and _2_28_x86_64 wheels and confirmed that libtbb is included in the x86_64 wheel but not the aarch64 wheel. Will investigate next week.

@tbirdso
Copy link
Contributor Author

tbirdso commented Jan 2, 2023

Successfully repaired, loaded, and used an ITKSplitComponents ARM wheel on a local ARM machine with the most recent patch. Will test in ITKSplitComponents CI.

EDIT: Testing at https://github.com/tbirdso/ITKSplitComponents/actions/runs/3823026183/jobs/6503756014

EDIT2: Verified that the ITKSplitComponents aarch64 resulting wheel is packaged with TBB and that the ITKSplitComponents example procedure (minus viewing) can be run on an aarch64 test machine. aarch64 build updates are good to go. 🟢

@tbirdso
Copy link
Contributor Author

tbirdso commented Jan 2, 2023

@dzenanz When you have a moment would you please re-review these changes before merge?

tbirdso and others added 4 commits January 2, 2023 11:22
Several quality-of-life fixes and updates for ARM support:
- Consolidates ARM module script with x86_64 module script for easier
  maintenance and comparison between procedures. Internal ARM script is
  preserved as a wrapper around generic internal build script.
- Splits `MANYLINUX_VERSION` variable into `MANYLINUX_VERSION` prefix
  and `TARGET_ARCH` postfix to better reflect available images.
- Uses `sudo` to avoid permissions failure when running docker or
  cleaning up files on ARM systems
- Improves cleanup step to avoid mixing x64 and ARM sources

co-authored-by: Matt McCormick <matt.mccormick@kitware.com>
Downgrades docker container image tag for compatibility with ITK build
archives built with gcc-11 toolset.

See tags at https://quay.io/repository/pypa/manylinux_2_28_aarch64?tab=tags
co-authored-by: Matt McCormick <matt.mccormick@kitware.com>
Addresses issue where TBB library was bundled with x64 but not ARM
module wheels.
@tbirdso
Copy link
Contributor Author

tbirdso commented Jan 2, 2023

@dzenanz Thanks! Moving forward with merge.

@tbirdso tbirdso merged commit cc3cbd2 into master Jan 2, 2023
tbirdso added a commit to InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction that referenced this pull request Jan 2, 2023
Exposes input parameter to allow the user to build for multiple Linux
toolsets and target architectures via available manylinux docker images.

Depends on ARM support introduced in ITKPythonPackage:
InsightSoftwareConsortium/ITKPythonPackage#236
tbirdso added a commit to InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction that referenced this pull request Jan 4, 2023
Exposes input parameter to allow the user to build for multiple Linux
toolsets and target architectures via available manylinux docker images.

Depends on ARM support introduced in ITKPythonPackage:
InsightSoftwareConsortium/ITKPythonPackage#236
@hjmjohnson hjmjohnson deleted the amend-arm-builds branch April 4, 2023 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build module wheels for ARM architecture
3 participants