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

Clean up CI script, add Mac arm64 and Ubuntu 22 builds #3946

Merged
merged 15 commits into from
Oct 24, 2024

Conversation

nickbianco
Copy link
Member

@nickbianco nickbianco commented Oct 19, 2024

Fixes issue #3944

Brief summary of changes

  • Updates continuous_integration.yml to include separate jobs for Mac x64 and Arm64 architectures.
  • Removed the "no Moco" Ubuntu job and replaced it with an Ubuntu 22 job.
  • Fixed a compiler error in WrapEllipsoid related to indexing a Mat44 that was causing the Ubuntu 22 job to fail.

Testing I've completed

Ran the CI, and ran both the Windows and Mac performance analysis workflows.

Looking for feedback on...

CHANGELOG.md (choose one)

  • no need to update because...CI updates.

Performance analysis

Platform: Mac, GitHub Actions runner

Test Name lhs [secs] stderr [secs] rhs [secs] stderr [secs] Speedup
Arm26 0.35 0.72 0.36 0.78 0.97
ToyDropLanding_function_based_paths 3.57 0.00 3.59 0.00 0.99
passive_dynamic_noanalysis 1.12 0.00 1.13 0.00 0.99
ToyDropLanding 3.92 0.00 3.93 0.00 1.00
Gait2354 0.21 0.00 0.21 0.00 1.00
ToyDropLanding_fbp_stepwisereg 3.63 0.00 3.63 0.00 1.00
passive_dynamic 1.71 0.00 1.71 0.00 1.00
MocoSquatToStand 3.45 0.00 3.46 0.00 1.00
ToyDropLanding_nomuscles 0.20 0.00 0.20 0.00 1.00
ellipsoid_wrap_function_based_paths 1.09 0.00 1.08 0.00 1.00
ellipsoid_wrap 1.28 0.00 1.28 0.00 1.00
MocoSlidingMass 0.57 0.00 0.57 0.00 1.00
RajagopalModel 3.14 0.00 3.14 0.00 1.00

This change is Reviewable

@aymanhab
Copy link
Member

aymanhab commented Oct 22, 2024

@nickbianco For the record the LD_LIBRARY_PATH changes appear to have been lost before the python tests are run on ubuntu. If everything else passes though then feel free to merge and I will take it on.

@nickbianco
Copy link
Member Author

@aymanhab, gotcha. As discussed, we can come back around to this later to resolve the Python testing issues on Ubuntu.

@nickbianco
Copy link
Member Author

Installing the Mac Arm64 build into a local conda environment fails. @aymanhab, thoughts? Looks like the rpaths are not being set correctly?

Python 3.10.15 (main, Oct  3 2024, 02:24:49) [Clang 14.0.6 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import opensim as osim
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/nbianco/miniconda3/envs/zmp/lib/python3.10/site-packages/opensim/__init__.py", line 18, in <module>
    from .simbody import *
  File "/Users/nbianco/miniconda3/envs/zmp/lib/python3.10/site-packages/opensim/simbody.py", line 10, in <module>
    from . import _simbody
ImportError: dlopen(/Users/nbianco/miniconda3/envs/zmp/lib/python3.10/site-packages/opensim/_simbody.so, 0x0002): Library not loaded: @rpath/libosimExampleComponents.dylib
  Referenced from: <35EB33C3-F08F-3BC8-A249-794230C5E39D> /Users/nbianco/miniconda3/envs/zmp/lib/python3.10/site-packages/opensim/_simbody.so
  Reason: tried: '/Users/nbianco/miniconda3/envs/zmp/lib/python3.10/site-packages/opensim/libosimExampleComponents.dylib' (no such file), '/Users/nbianco/miniconda3/envs/zmp/lib/python3.10/site-packages/opensim/../../../sdk/lib/libosimExampleComponents.dylib' (no such file), '/Users/runner/work/opensim-core/opensim-core-install/sdk/lib/libosimExampleComponents.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/runner/work/opensim-core/opensim-core-install/sdk/lib/libosimExampleComponents.dylib' (no such file), '/Users/nbianco/miniconda3/envs/zmp/lib/python3.10/site-packages/opensim/../../../sdk/Simbody/lib/libosimExampleComponents.dylib' (no such file), '/Users/runner/work/opensim-core/opensim-core-install/sdk/Simbody/lib/libosimExampleComponents.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/runner/work/opensim-core/opensim-core-install/sdk/Simbody/lib/libosimExampleComponents.dylib' (no such file), '/Users/nbianco/miniconda3/envs/zmp/lib/python3.10/site-packages/opensim/libosimExampleComponents.dylib' (no such file), '/Users/nbianco/miniconda3/envs/zmp/lib/python3.10/site-packages/opensim/../../../sdk/lib/libosimExampleComponents.dylib' (no such file), '/Users/runner/work/opensim-core/opensim-core-install/sdk/lib/libosimExampleComponents.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/runner/work/opensim-core/opensim-core-install/sdk/lib/libosimExampleComponents.dylib' (no such file), '/Users/nbianco/miniconda3/envs/zmp/lib/python3.10/site-packages/opensim/../../../sdk/Simbody/lib/libosimExampleComponents.dylib' (no such file), '/Users/runner/work/opensim-core/opensim-core-install/sdk/Simbody/lib/libosimExampleComponents.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/runner/work/opensim-core/opensim-core-install/sdk/Simbody/lib/libosimExampleComponents.dylib' (no such file), '/Users/nbianco/miniconda3/envs/zmp/bin/../lib/libosimExampleComponents.dylib' (no such file), '/Users/nbianco/miniconda3/envs/zmp/bin/../lib/libosimExampleComponents.dylib' (no such file)

@aymanhab
Copy link
Member

@nickbianco Conda makes some assumptions about layout. When we build conda package, the conda build system modifies rpaths accordingly. I'd test installing in non conda environment using setup.py etc. but not sure conda would work out of the box. I'd also test java/matlab
Just let me know what you want me to try/test to get this over the finish line

@nickbianco
Copy link
Member Author

Yeah, it looks like python -m pip install . doesn't install the .dylibs anywhere in the conda environment. I'll try a different install approach and see what I get.

@nickbianco
Copy link
Member Author

Using a Python venv fails too: nothing is copied into the /lib folder of the venv.

@nickbianco nickbianco marked this pull request as ready for review October 24, 2024 16:51
@nickbianco
Copy link
Member Author

@aymanhab, this is ready for review. I didn't test the "Build GUI" workflow; let me know if you'd like that tested as well.

@aymanhab
Copy link
Member

Would be good to test just for completeness though it has no dependencies on ci scripts.

@nickbianco
Copy link
Member Author

@aymanhab, the GUI build job fails, I think because a personal access token needs to be added or updated. It's up to you if you consider that blocking here.

@aymanhab
Copy link
Member

Thanks for trying it out @nickbianco Definitely not blocking, will review accordingly.

Copy link
Member

@aymanhab aymanhab left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 2 files at r1, 1 of 1 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nickbianco)

@nickbianco
Copy link
Member Author

As discussed, the Ubuntu 22 failure is unrelated and the GUI build job will be resolved in a future PR.

Thanks @aymanhab!

@nickbianco nickbianco merged commit 897689f into main Oct 24, 2024
11 of 13 checks passed
@nickbianco nickbianco deleted the ci_arm64_mac_support branch October 24, 2024 21:25
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.

2 participants