Skip to content

Conversation

@EricaCMitchell
Copy link

Description

Eigen 5.0.0 changes where version is located. Needs to find version header for Eigen 5.x.x and Eigen 3.x.x. Same change as Libint PR 395.

Details

  • Changes FindEigen3.cmake to try finding version for Eigen 5.x.x in new file Eigen/Version or try finding version for Eigen 3.x.x in old location Eigen/src/Core/util/Macros.h

Copy link

Copilot AI left a comment

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 updates FindEigen3.cmake to support both Eigen 5.x and 3.x version detection, as the version header location changed in Eigen 5.0.0. However, the PR also includes several unrelated changes in test files.

Changes:

  • Updated FindEigen3.cmake to check for version headers in both Eigen 5.x location (Eigen/Version) and Eigen 3.x location (Eigen/src/Core/util/Macros.h)
  • Refactored constructors in tests/sparse_tile.h to use IIFE pattern for running assertions before member initialization
  • Modified test in tests/expressions_impl.h to use a temporary variable instead of reusing the same array
  • Fixed assertions in src/TiledArray/tensor/tensor_interface.h to check member variables instead of parameters
  • Changed spacing around archive operators in tests/sparse_tile.h

Reviewed changes

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

File Description
cmake/modules/FindEigen3.cmake Adds support for Eigen 5.x version header location with fallback to Eigen 3.x location
tests/sparse_tile.h Refactors constructors to use IIFE pattern and modifies archive operator spacing
tests/expressions_impl.h Changes test to use temporary variable for block operations
src/TiledArray/tensor/tensor_interface.h Fixes assertions to check member variables after initialization
Comments suppressed due to low confidence (1)

cmake/modules/FindEigen3.cmake:54

  • After reading the version header file, if the regex matches fail (lines 47-52), the version variables (EIGEN3_WORLD_VERSION, EIGEN3_MAJOR_VERSION, EIGEN3_MINOR_VERSION) will be empty strings. This could lead to an incorrect EIGEN3_VERSION being set on line 54 (e.g., "..") and potentially unexpected behavior in version comparison on line 55. Consider adding validation after the regex matches to ensure the version numbers were successfully extracted, or set EIGEN3_VERSION_OK to FALSE if any of the version components are empty.
  string(REGEX MATCH "define[ \t]+EIGEN_WORLD_VERSION[ \t]+([0-9]+)" _eigen3_world_version_match "${_eigen3_version_header}")
  set(EIGEN3_WORLD_VERSION "${CMAKE_MATCH_1}")
  string(REGEX MATCH "define[ \t]+EIGEN_MAJOR_VERSION[ \t]+([0-9]+)" _eigen3_major_version_match "${_eigen3_version_header}")
  set(EIGEN3_MAJOR_VERSION "${CMAKE_MATCH_1}")
  string(REGEX MATCH "define[ \t]+EIGEN_MINOR_VERSION[ \t]+([0-9]+)" _eigen3_minor_version_match "${_eigen3_version_header}")
  set(EIGEN3_MINOR_VERSION "${CMAKE_MATCH_1}")

  set(EIGEN3_VERSION ${EIGEN3_WORLD_VERSION}.${EIGEN3_MAJOR_VERSION}.${EIGEN3_MINOR_VERSION})

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@EricaCMitchell EricaCMitchell force-pushed the emitchell/upgrade/eigen-5.x.x branch from 486a095 to ad4bfd1 Compare February 3, 2026 23:07
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