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

Bug: Example Framework does not compile in an environment without TBB #3610

Open
tomeichlersmith opened this issue Sep 12, 2024 · 4 comments · May be fixed by #3507
Open

Bug: Example Framework does not compile in an environment without TBB #3610

tomeichlersmith opened this issue Sep 12, 2024 · 4 comments · May be fixed by #3507
Labels

Comments

@tomeichlersmith
Copy link

This is not a critical bug but is a usability obstacle for downstream ACTS users. I chose to label this a bug since the CMakeLists.txt including this as an option implies to me that the examples should be able to build without TBB.

I am building ACTS within a fixed environment where TBB is unavailable.
I configure ACTS with

cmake -B acts/build -S acts -DACTS_BUILD_EXAMPLES=ON -DACTS_USE_EXAMPLES_TBB=OFF

and then build with

cmake --build acts/build -- -j4

I have uploaded the full command log for review, but the relevant parts are...

during the configuration step, ACTS acknowledges that it will be built in single-threaded mode.

-- Ignore subdirectory 'Examples/Detectors/MuonSpectrometerMockupDetector'
-- disable TBB for Examples/Framework - only single-threaded running will be supported
-- Ignore subdirectory 'Examples/Framework/ML'

the build of the examples fails when trying to include a tbb header

[ 56%] Building CXX object Examples/Framework/CMakeFiles/ActsExamplesFramework.dir/src/Framework/RandomNumbers.cpp.o
[ 57%] Building CXX object Examples/Framework/CMakeFiles/ActsExamplesFramework.dir/src/Framework/Sequencer.cpp.o
In file included from /home/tom/code/ldmx/acts-tbb/acts/Examples/Framework/src/Framework/Sequencer.cpp:9:
/home/tom/code/ldmx/acts-tbb/acts/Examples/Framework/include/ActsExamples/Framework/Sequencer.hpp:30:10: fatal error: tbb/enumerable_thread_specific.h: No such file or directory
   30 | #include <tbb/enumerable_thread_specific.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
gmake[2]: *** [Examples/Framework/CMakeFiles/ActsExamplesFramework.dir/build.make:174: Examples/Framework/CMakeFiles/ActsExamplesFramework.dir/src/Framework/Sequencer.cpp.o] Error 1
gmake[2]: *** Waiting for unfinished jobs....

I'm guessing this is simply an issue with not wrapping the relevant TBB parts in the Sequencer. For example,

#include <tbb/enumerable_thread_specific.h>

Sequencer::Sequencer(const Sequencer::Config& cfg)
: m_cfg(cfg),
m_taskArena((m_cfg.numThreads < 0) ? tbb::task_arena::automatic
: m_cfg.numThreads),
m_logger(Acts::getDefaultLogger("Sequencer", m_cfg.logLevel)) {
#ifndef ACTS_EXAMPLES_NO_TBB
if (m_cfg.numThreads == 1) {
#endif

We are proceeding forward with simply commenting out the Sequencer compilation from the framework example since we are just trying to get to using the CSV writer (which depends on the writer interface within the framework example).

diff --git a/Examples/Framework/CMakeLists.txt b/Examples/Framework/CMakeLists.txt
index 692db8f2f..8754d3bde 100644
--- a/Examples/Framework/CMakeLists.txt
+++ b/Examples/Framework/CMakeLists.txt
@@ -12,7 +12,7 @@ add_library(
     src/Framework/SequenceElement.cpp
     src/Framework/WhiteBoard.cpp
     src/Framework/RandomNumbers.cpp
-    src/Framework/Sequencer.cpp
+    #src/Framework/Sequencer.cpp
     src/Utilities/EventDataTransforms.cpp
     src/Utilities/Paths.cpp
     src/Utilities/Options.cpp
@andiwand
Copy link
Contributor

I think this is expected see #3507

@paulgessinger
Copy link
Member

We didn't end up removing the flag in that PR, but maybe this is reason to do that.
The alternative is to use preprocessor ifs to remove the code that uses tbb conditionally.

I would argue we can just accept that tbb is required for the examples and move on.

@tomeichlersmith
Copy link
Author

That works for me! As I mentioned, the only reason we were confused is because it seemed like it was optional. :)

That PR is still open, should this decision be put into that PR or should a separate PR be opened?

@AJPfleger AJPfleger linked a pull request Sep 24, 2024 that will close this issue
Copy link

This issue/PR has been automatically marked as stale because it has not had recent activity. The stale label will be removed if any interaction occurs.

@github-actions github-actions bot added the Stale label Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants