Skip to content

Patch eigen 3.4.0 to make consistent with source used in ORT 1.16 release - #18320

Closed
Scott McKay (skottmckay) wants to merge 3 commits into
mainfrom
skottmckay/Patch_Eigen_3_4_0_to_make_consistent_with_source_used_in_1_16_release
Closed

Patch eigen 3.4.0 to make consistent with source used in ORT 1.16 release#18320
Scott McKay (skottmckay) wants to merge 3 commits into
mainfrom
skottmckay/Patch_Eigen_3_4_0_to_make_consistent_with_source_used_in_1_16_release

Conversation

@skottmckay

Copy link
Copy Markdown
Contributor

Description

Add patch to make the Eigen source the same as what was used in the 1.16.x releases.

The patch is large, but makes things consistent.

At a minimum the changes to Eigen\src\Core\PartialReduxEvaluator.h are required to fix the arm64 Apple builds.

By using the same patch for all builds it will hopefully also fix the issue with ACL builds. #15248
The patch command for ACL builds has arguments that are specific to git apply, but I don't believe (please correct me if I'm wrong) we can use that when we download the source as a zip instead of clone it using git.

Motivation and Context

Fix arm64 macOS/iOS build error due to inconsistencies between the official Eigen 3.4.0 source and what was previously downloaded from the 3.4 branch.

@skottmckay
Scott McKay (skottmckay) marked this pull request as ready for review November 7, 2023 10:25
@skottmckay Scott McKay (skottmckay) changed the title Patch eigen 3 4 0 to make consistent with source used in 1 16 release Patch eigen 3.4.0 to make consistent with source used in ORT 1.16 release Nov 7, 2023
@Timen

Copy link
Copy Markdown

Isn't it a better idea to just pull the actual commit that had been used up until the most recent changes eg:
https://gitlab.com/libeigen/eigen/-/archive/e7248b26a1ed53fa030c5c459f7ea095dfd276ac/eigen-e7248b26a1ed53fa030c5c459f7ea095dfd276ac.zip ?

This would only require a hash change but only includes the changes from may 19 2022 which were the most recent ones up until june 7 2023.

@skottmckay

Scott McKay (skottmckay) commented Nov 7, 2023

Copy link
Copy Markdown
Contributor Author

Isn't it a better idea to just pull the actual commit that had been used up until the most recent changes eg: https://gitlab.com/libeigen/eigen/-/archive/e7248b26a1ed53fa030c5c459f7ea095dfd276ac/eigen-e7248b26a1ed53fa030c5c459f7ea095dfd276ac.zip ?

This would only require a hash change but only includes the changes from may 19 2022 which were the most recent ones up until june 7 2023.

It would be. Where is that commit id specified?

The files look to be the same. The hash differs from the archive we were previously using but maybe that doesn't matter and there was a difference in compression level.

eigen;https://gitlab.com/libeigen/eigen/-/archive/3.4/eigen-3.4.zip;ee201b07085203ea7bd8eb97cbcb31b07cfa3efb

(Get-FileHash -Algorithm SHA1 eigen-e7248b26a1ed53fa030c5c459f7ea095dfd276ac.zip).Hash.ToLower()
be8be39fdbc6e60e94fa7870b280707069b5b81a

@Timen

Copy link
Copy Markdown

That commit ID is taken from the history of the 3.4 branch. And includes the latest changes up to when eigen was pulled in using CMake fetchcontent instead of submodule.

@skottmckay

Copy link
Copy Markdown
Contributor Author

Closing in favour of #18200 which uses the cleaner option of a commit id.

I'll split out the updates so patch runs on the Windows devops machines into a separate PR.

@skottmckay
Scott McKay (skottmckay) deleted the skottmckay/Patch_Eigen_3_4_0_to_make_consistent_with_source_used_in_1_16_release branch September 5, 2024 23:12
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