Skip to content

Support for Eigen3 5.0.0#651

Merged
egull merged 1 commit into
ALPSCore:2.3.2from
cho-m:eigen5
Oct 5, 2025
Merged

Support for Eigen3 5.0.0#651
egull merged 1 commit into
ALPSCore:2.3.2from
cho-m:eigen5

Conversation

@cho-m

@cho-m cho-m commented Oct 4, 2025

Copy link
Copy Markdown
Contributor

Note that Eigen 5.0.0 is still Eigen3 (EIGEN_WORLD_VERSION=3) but just uses their new version scheme:

Changes account for:

  • Latest Eigen has moved the version information to Eigen/Version - https://gitlab.com/libeigen/eigen/-/blob/5.0.0/Eigen/Version
  • A build failure I hit: 
    .../accumulators/src/mpi.cpp:56:21: error: use of undeclared identifier 'assert'
   56 |                     assert(buffer.size()>=offset+size && "buffer has sufficient size to accommodate values");
      |                     ^

@cho-m cho-m mentioned this pull request Oct 4, 2025
Merged
23 tasks
@cho-m

cho-m commented Oct 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Looks like original approach will set version as 3.5.0 since it always starts with world part.

EDIT: Updated to extract major, minor patch from header. Build is successful and includes set(ALPSCore_HAS_EIGEN_VERSION "5.0.0") in files

Tried local builds using both Eigen 3.4.0 and 5.0.0 with args -DALPS_CXX_STD=c++14 -DTesting=ON -DEIGEN_INCLUDE_DIR=....

Both had same results with only 1 failed test (TensorTest.TestMoveAssignments2) so doesn't seem to have any noticeable API breakage in 5.0.0.

@cho-m cho-m changed the title support for eigen 5.0.0 Support for Eigen3 5.0.0 Oct 4, 2025
@egull

egull commented Oct 5, 2025

Copy link
Copy Markdown
Collaborator

Thanks. Could you please split the two requests? The cassert header is unrelated to the FindEigen and we can just merge it.

The FindEigen requires some more discussion. I see the following issues:

  1. FindEigen has been provided by cmake for some time. It may make sense to use that version instead of maintaining our own and committing the fix. Would that work? Any advantage / disadvantage?
  2. Eigen3 version 5 requires c++-14 rather than c++-11. We'd have to modify our compiler tests or increase the requirement to c++-14. I'm not sure if anybody still has a need for only c++-11. Comments?

Any opinions?

@cho-m

cho-m commented Oct 5, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author
  • FindEigen has been provided by cmake for some time. It may make sense to use that version instead of maintaining our own and committing the fix. Would that work? Any advantage / disadvantage?

I don't think CMake provides FindEigen. Eigen does ship a Eigen3Config.cmake which is used by ALPSCore when EIGEN3_INCLUDE_DIR is not set.

However, in 5.0.0, Eigen removed EIGEN3_INCLUDE_DIR variable and now expects only Eigen3::Eigen target usage - https://gitlab.com/libeigen/eigen/-/commit/f2984cd0778dd0a1d7e74216d826eaff2bc6bfab

Also need to account for version checking - https://gitlab.com/libeigen/eigen/-/merge_requests/1989/diffs, e.g.

  • find_package(Eigen3) would find 3.4.0, 3.4.1 and 5.0.0 (and every other version available)
  • find_package(Eigen3 3.4) would only find 3.4.0 and 3.4.1
  • find_package(Eigen3 3.4....5) would only find 3.4.1 and 5.0.0

@cho-m cho-m mentioned this pull request Oct 5, 2025
Merged
@egull egull requested a review from Copilot October 5, 2025 17:02
Copilot AI reviewed Oct 5, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for Eigen3 version 5.0.0, which introduced a new versioning scheme while maintaining EIGEN_WORLD_VERSION=3. The changes handle the relocation of version information to a new header file and fix a missing include that caused compilation errors.

  • Updated CMake FindEigen3 module to handle version detection from both new and legacy locations
  • Added missing #include <cassert> to fix compilation error in MPI code

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
common/cmake/FindEigen3.cmake Added fallback logic to read version info from new Eigen/Version header while maintaining backward compatibility
accumulators/src/mpi.cpp Added missing cassert include to resolve compilation error with assert macro

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread common/cmake/FindEigen3.cmake
set(EIGEN3_MINOR_VERSION "${CMAKE_MATCH_1}")

set(EIGEN3_VERSION ${EIGEN3_WORLD_VERSION}.${EIGEN3_MAJOR_VERSION}.${EIGEN3_MINOR_VERSION})
endif(EXISTS "${EIGEN3_INCLUDE_DIR}/Eigen/Version")

Copilot AI Oct 5, 2025

Copy link

Choose a reason for hiding this comment

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

Use modern CMake syntax by removing the condition from the endif() statement. Modern CMake style uses just endif() without repeating the condition.

Copilot uses AI. Check for mistakes.
@egull egull merged commit 618c6ef into ALPSCore:2.3.2 Oct 5, 2025
4 of 13 checks passed
@cho-m cho-m deleted the eigen5 branch October 6, 2025 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cho-m @egull