xsimd 8 is not supported
Closed this issue · 13 comments
cpp/CMakeLists.txt declares xsimd support with
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.19)
find_package(xsimd ${XSIMD_MIN_VERSION}...<8.0 QUIET)
The <8.0 means that basix cannot be build against xsimd 8. Is this constraint still intended now? xsimd 8 was released last month.
If I understand correctly, basix itself isn't using xsimd. XTENSOR_USE_XSIMD is used only in xtensor,
xtensor itself checks for versioned xsimd in cmake/xtensor/xtensorConfig.cmake, e.g. for xtensor 0.24.0
if(XTENSOR_USE_XSIMD)
find_dependency(xsimd 8.0.2)
target_link_libraries(xtensor INTERFACE xsimd)
target_compile_definitions(xtensor INTERFACE XTENSOR_USE_XSIMD)
endif()
As far as I can tell, that means basix does not need find_package(xsimd) at all. Just needs to set XTENSOR_USE_XSIMD before calling find_package(xtensor). Same goes for dolfinx.
Should we remove all find_package(xsimd) from basix and dolfinx?
Correct, Basix doesn't use xsimd directly. But, Basix does need to know if xtensor has used xsimd otherwise we run into API/ABI compatibility issues with libraries that call the Basix API.
Makes sense. Seems to me it should be possible to extract that from xtensor itself though. Possibly by patching xtensor/xtensorConfig.cmake to set a relevant variable (in xtensor upstream I mean)
I think I see some of the challenge. find_package(xtensor) does itself look for xsimd and sets xsimd_FOUND, xsimd_VERSION, etc. The question is what to do about it if xtensor did not find the xsimd that it needs.
It's partly a question of build philosophy. We could have the build just stop if the required component is not found. Then the build environment could be fixed to make sure it's installed. The method currently in place, though, sets CMakeFiles.txt to download source if the component was not found.
One question then: if xtensor is found, but not the xsimd that it needs, would it be sufficient to run FetchContent_Declare for xsimd after finding xtensor, using an xsimd version obtained from xtensor?
I'd be inclined to remove xsimd as an optional dependency. We don't have evidence as to the performance advantages (hasn't been carefully studied), and it can make interaction with other libraries fragile.
It's an interesting question whether xsimd helps. On my to-do list I'm planning to set up performance testing of the debian packages (running fenicsx-performance-tests). I intend to include testing over various permutations, e.g. different BLAS. If possible, I'll try to set up tests with and without xsimd.
Incidently I just noticed there's the same problem in basix CMakeLists.txt in regards to the xtensor version, but with a contradiction in the requirements. It's also got an upper-version constraint,
find_package(xtensor ${XTENSOR_MIN_VERSION}...<0.24 QUIET)
but if that fails (doesn't find xtensor 0.23.10, i.e. if 0.24 is already installed), then it fetches 0.24,
FetchContent_Declare(
xtensor
GIT_REPOSITORY https://github.com/xtensor-stack/xtensor.git
GIT_TAG 0.24.0
)
There is some pretty complex logic in the current CMakeLists.txt. Another subtle issue the potential to get a downloaded xsimd that is incompatible with the user installed xtensor and xtl.
I'd definitely be in favour of removing xsimd as a dependency, and also I would like to see the automatic git downloads removed.
At the very least, the download should be explicitly defined by a configuration option e.g. DOWNLOAD_XLIBS. Then it should either look for system installed libraries and terminate on not found, or (optionally) download a good complete set of XLIBS.
I have a PR, but I'm out and about for the next couple of hours. Don't waste time repeating it.
Not sure if you meant "DOWNLOAD_XLIBS" literally or just shorthand while out and about. I'd suggest not calling it "XLIBS" since that tends to imply Xlib with libX11.so etc, the libs for the X server display. Maybe XMATHLIBS instead? Or XTENSORLIBS, or XTLIBS.
DOWNOAD_XTENSOR_LIBS?