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

Fix support for oneTBB new inteface >= v2021.1 #2974

Closed
wants to merge 5 commits into from
Closed

Fix support for oneTBB new inteface >= v2021.1 #2974

wants to merge 5 commits into from

Conversation

jeanchristopheruel
Copy link

Submitting a support for oneTBB new interface. I suggest that this should be tested with a proper CI/CD.
oneTBB library do not provide <tbb/tbb_stddef.h> anymore as can be seen here. It is now replaced by <tbb/tbb.h> since v2021.1.

@WardBrian
Copy link
Member

I believe __has_include is only available for C++17, while Stan still supports C++14 as a minimum.

Are you defining TBB_INTERFACE_NEW during compilation? That is required to use Stan-math with TBB 2021

Jean-Christophe Ruel and others added 2 commits November 20, 2023 13:45
__has_include(<tbb/tbb_stddef.h>) -> TBB_VERSION_MAJOR >= 2021
@jeanchristopheruel
Copy link
Author

jeanchristopheruel commented Nov 20, 2023

Instead of relying on the makefile, I use CMake's FetchContent to integrate Stan-math into my project as a module. This approach streamlines the build process and eliminates the need to install unused components, allowing greater flexibility in managing dependencies.

I could find related interest in using FetchContent + CMakeList.txt in #2967

include(FetchContent)
FetchContent_Declare(
     stanmath
     GIT_REPOSITORY https://github.com/jeanchristopheruel/math
     GIT_TAG develop
     GIT_SHALLOW TRUE
     )
FetchContent_MakeAvailable(stanmath)

find_package(Boost 1.81.0 REQUIRED)
find_package(TBB 2021 REQUIRED)
find_package(Eigen3 3.4 REQUIRED)

add_library(stan-math INTERFACE)
target_link_libraries(stan-math INTERFACE Eigen3::Eigen TBB::tbb -lstdc++ -lm)
target_include_directories(stan-math INTERFACE ${stanmath_SOURCE_DIR} ${Boost_INCLUDE_DIRS})
add_library(stan::math ALIAS stan-math)

@WardBrian
Copy link
Member

#2967 is definitely the right place to discuss CMake, and I'm hoping we can support it sooner rather than later.

In the mean time, you should still be using target_compile_definitions or just add_compile_options to define TBB_INTERFACE_NEW as the way of selecting to use newer versions

@jeanchristopheruel
Copy link
Author

Thanks for your time. I will do so. However, it still wont support TBB_VERSION_MAJOR == 2021. Current statement for #if TBB_VERSION_MAJOR >= 2020 will fail.

@WardBrian
Copy link
Member

We use TBB 2021 for the conda-forge builds of cmdstan with this. If you can share the error you are getting I can try to help debug it, but it is definitely possible to use newer TBBs without source changes

@jeanchristopheruel
Copy link
Author

jeanchristopheruel commented Nov 20, 2023

OneApi TBB does not have tbb_stddef.h anymore. TBB might?
Migration guide TBB -> OneApi TBB

@WardBrian
Copy link
Member

If you define TBB_INTERFACE_NEW, that header will not be included by Stan Math

@jeanchristopheruel
Copy link
Author

Indeed, as you previously pointed out. My apologies.
Closing issue.

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.

3 participants