acts-project / acts

Experiment-independent toolkit for (charged) particle track reconstruction in (high energy) physics experiments implemented in modern C++
https://acts.readthedocs.io
Mozilla Public License 2.0
105 stars 168 forks source link

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

Closed tomeichlersmith closed 1 week ago

tomeichlersmith commented 1 month ago

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,

https://github.com/acts-project/acts/blob/d250bbfaa33b47a873307a3d7c65e2553f442c3f/Examples/Framework/include/ActsExamples/Framework/Sequencer.hpp#L29-L31

https://github.com/acts-project/acts/blob/d250bbfaa33b47a873307a3d7c65e2553f442c3f/Examples/Framework/src/Framework/Sequencer.cpp#L106-L113

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 commented 1 month ago

I think this is expected see https://github.com/acts-project/acts/pull/3507

paulgessinger commented 1 month ago

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 commented 1 month ago

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?

github-actions[bot] commented 3 weeks ago

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.