build: make lda optional with compile feature (VowpalWabbit#4555)

* build: make lda optional with compile feature * formatting * Create README * add to python vcpkg * dont add boost dep * try fix python * add comment --------- Co-authored-by: Eduardo Salinas <[email protected]>
bassmang · Apr 6, 2023 · 8b9a750 · 8b9a750
1 parent 6ee7721
commit 8b9a750
Show file tree

Hide file tree

Showing 10 changed files with 76 additions and 34 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -54,6 +54,12 @@ if(VW_FEAT_FLATBUFFERS)
   list(APPEND VCPKG_MANIFEST_FEATURES "flatbuffers")
 endif()
 
+# This is a hack around the fact the Python build does not seem to be capable of
+# dealing with manifest features.
+if(VW_FEAT_LDA AND NOT BUILD_PYTHON)
+  list(APPEND VCPKG_MANIFEST_FEATURES "lda")
+endif()
+
 option(BUILD_TESTING "Build tests" ON)
 if(BUILD_TESTING)
   list(APPEND VCPKG_MANIFEST_FEATURES "tests")

diff --git a/cmake/VowpalWabbitFeatures.cmake b/cmake/VowpalWabbitFeatures.cmake
@@ -8,17 +8,12 @@
 # If a feature called X is enabled then:
 #   - The cmake variable VW_FEAT_X is set to ON, otherwise it is OFF
 #   - The C++ macro VW_FEAT_X_ENABLED is defined if the feature is enabled, otherwise it is not defined
-#
-# Features:
-#   FLATBUFFERS: Enable flatbuffers support
-#   CSV: Enable csv parser
-#   LAS_SIMD: Enable large action space with explicit simd (only work with linux for now)
 
-# set(VW_ALL_FEATURES "LAS_SIMD;FLATBUFFERS;CSV")
-set(VW_ALL_FEATURES "CSV;FLATBUFFERS")
+set(VW_ALL_FEATURES "CSV;FLATBUFFERS;LDA")
 
 option(VW_FEAT_FLATBUFFERS "Enable flatbuffers support" OFF)
 option(VW_FEAT_CSV "Enable csv parser" OFF)
+option(VW_FEAT_LDA "Enable lda reduction" ON)
 # option(VW_FEAT_LAS_SIMD "Enable large action space with explicit simd (only works with linux for now)" ON)
 
 # Legacy options for feature enablement

diff --git a/ext_libs/README b/ext_libs/README
@@ -0,0 +1,9 @@
+boost_math - optional
+eigen - optional
+fmt
+rapidjson
+spdlog
+sse2neon - optional
+string-view-lite
+vcpkg
+zlib
diff --git a/ext_libs/ext_libs.cmake b/ext_libs/ext_libs.cmake
@@ -44,13 +44,16 @@ else()
   target_include_directories(RapidJSON SYSTEM INTERFACE "${CMAKE_CURRENT_LIST_DIR}/rapidjson/include")
 endif()
 
-if(VW_BOOST_MATH_SYS_DEP)
-  find_package(Boost REQUIRED)
-  set(boost_math_target Boost::boost)
-else()
-  set(BOOST_MATH_STANDALONE ON CACHE BOOL "Use Boost math vendored dep in standalone mode" FORCE)
-  add_subdirectory(${CMAKE_CURRENT_LIST_DIR}/boost_math EXCLUDE_FROM_ALL)
-  set(boost_math_target Boost::math)
+# Boost math only required if LDA is enabled.
+if(VW_FEAT_LDA)
+  if(VW_BOOST_MATH_SYS_DEP)
+    find_package(Boost REQUIRED)
+    set(boost_math_target Boost::boost)
+  else()
+    set(BOOST_MATH_STANDALONE ON CACHE BOOL "Use Boost math vendored dep in standalone mode" FORCE)
+    add_subdirectory(${CMAKE_CURRENT_LIST_DIR}/boost_math EXCLUDE_FROM_ALL)
+    set(boost_math_target Boost::math)
+  endif()
 endif()
 
 if(VW_ZLIB_SYS_DEP)

diff --git a/python/vcpkg_default.json b/python/vcpkg_default.json
@@ -1,6 +1,12 @@
 {
-    "name": "demo",
-    "version": "1.0.0",
-    "builtin-baseline": "773516ecf6014d89cc69b11bb54605ad4be56694",
-    "dependencies": [ "zlib","boost-python","python3","flatbuffers" ]
-}
+  "name": "demo",
+  "version": "1.0.0",
+  "builtin-baseline": "773516ecf6014d89cc69b11bb54605ad4be56694",
+  "dependencies": ["zlib", "boost-python", "python3", "flatbuffers"],
+  "features": {
+    "lda": {
+      "description": "",
+      "dependencies": []
+    }
+  }
+}
diff --git a/python/vcpkg_python310.json b/python/vcpkg_python310.json
@@ -1,9 +1,15 @@
 {
-    "name": "demo",
-    "version": "1.0.0",
-    "builtin-baseline": "773516ecf6014d89cc69b11bb54605ad4be56694",
-    "dependencies": [ "zlib","boost-python","python3","flatbuffers" ],
-    "overrides": [
-        { "name": "python3", "version-string": "3.10.2", "port-version": 1 }
-    ]
-}
+  "name": "demo",
+  "version": "1.0.0",
+  "builtin-baseline": "773516ecf6014d89cc69b11bb54605ad4be56694",
+  "dependencies": ["zlib", "boost-python", "python3", "flatbuffers"],
+  "overrides": [
+    { "name": "python3", "version-string": "3.10.2", "port-version": 1 }
+  ],
+  "features": {
+    "lda": {
+      "description": "",
+      "dependencies": []
+    }
+  }
+}
diff --git a/vcpkg.json b/vcpkg.json
@@ -3,9 +3,7 @@
   "name": "vowpal-wabbit",
   "version": "9.8.0",
   "dependencies": [
-    "boost-math",
     "eigen3",
-    "flatbuffers",
     "fmt",
     "rapidjson",
     "spdlog",
@@ -17,6 +15,10 @@
       "description": "Build Tests",
       "dependencies": ["gtest"]
     },
+    "lda": {
+      "description": "Enable LDA reduction",
+      "dependencies": ["boost-math"]
+    },
     "flatbuffers": {
       "description": "Enable flatbuffers support",
       "dependencies": ["flatbuffers"]

diff --git a/vowpalwabbit/core/CMakeLists.txt b/vowpalwabbit/core/CMakeLists.txt
@@ -140,7 +140,6 @@ set(vw_core_headers
   include/vw/core/reductions/interact.h
   include/vw/core/reductions/interaction_ground.h
   include/vw/core/reductions/kernel_svm.h
-  include/vw/core/reductions/lda_core.h
   include/vw/core/reductions/log_multi.h
   include/vw/core/reductions/lrq.h
   include/vw/core/reductions/lrqfa.h
@@ -314,7 +313,6 @@ set(vw_core_sources
   src/reductions/interact.cc
   src/reductions/interaction_ground.cc
   src/reductions/kernel_svm.cc
-  src/reductions/lda_core.cc
   src/reductions/log_multi.cc
   src/reductions/lrq.cc
   src/reductions/lrqfa.cc
@@ -361,6 +359,11 @@ set(vw_core_sources
   src/vw.cc
 )
 
+if(VW_FEAT_LDA)
+  list(APPEND vw_core_headers include/vw/core/reductions/lda_core.h)
+  list(APPEND vw_core_sources src/reductions/lda_core.cc)
+endif()
+
 vw_add_library(
     NAME "core"
     TYPE "STATIC_ONLY"
@@ -370,13 +373,17 @@ vw_add_library(
         # Special case for dealing with header-only INTERFACE dependencies of a STATIC library
         # Use BUILD_INTERFACE to prevent them from being exported, i.e. treat them as PRIVATE
         # https://gitlab.kitware.com/cmake/cmake/issues/15415
-        ${CMAKE_DL_LIBS} ${LINK_THREADS} vw_io $<BUILD_INTERFACE:${boost_math_target}> $<BUILD_INTERFACE:RapidJSON>
+        ${CMAKE_DL_LIBS} ${LINK_THREADS} vw_io $<BUILD_INTERFACE:RapidJSON>
         $<BUILD_INTERFACE:eigen> $<BUILD_INTERFACE:sse2neon>
     DESCRIPTION "This contains all remaining VW code, all reduction implementations, driver, option handling"
     EXCEPTION_DESCRIPTION "Yes"
     ENABLE_INSTALL
 )
 
+if(VW_FEAT_LDA)
+  target_link_libraries(vw_core PRIVATE $<BUILD_INTERFACE:${boost_math_target}>)
+endif()
+
 target_include_directories(vw_core PRIVATE ${CMAKE_CURRENT_LIST_DIR}/src)
 
 if (VW_BUILD_LAS_WITH_SIMD AND (UNIX AND NOT APPLE) AND (${CMAKE_SYSTEM_PROCESSOR} STREQUAL "x86_64"))

diff --git a/vowpalwabbit/core/include/vw/core/reductions/lda_core.h b/vowpalwabbit/core/include/vw/core/reductions/lda_core.h
@@ -9,6 +9,8 @@
 
 #include <memory>
 
+#ifdef VW_FEAT_LDA_ENABLED
+
 namespace VW
 {
 namespace reductions
@@ -19,4 +21,6 @@ namespace lda
 void get_top_weights(VW::workspace* all, int top_words_count, int topic, std::vector<feature>& output);
 }
 }  // namespace reductions
-}  // namespace VW
+}  // namespace VW
+
+#endif
diff --git a/vowpalwabbit/core/src/reduction_stack.cc b/vowpalwabbit/core/src/reduction_stack.cc
@@ -65,7 +65,9 @@
 #include "vw/core/reductions/interact.h"
 #include "vw/core/reductions/interaction_ground.h"
 #include "vw/core/reductions/kernel_svm.h"
-#include "vw/core/reductions/lda_core.h"
+#ifdef VW_FEAT_LDA_ENABLED
+#  include "vw/core/reductions/lda_core.h"
+#endif
 #include "vw/core/reductions/log_multi.h"
 #include "vw/core/reductions/lrq.h"
 #include "vw/core/reductions/lrqfa.h"
@@ -163,7 +165,9 @@ void prepare_reductions(std::vector<std::tuple<std::string, VW::reduction_setup_
   reductions.push_back(VW::reductions::lrqfa_setup);
   reductions.push_back(VW::reductions::stagewise_poly_setup);
   reductions.push_back(VW::reductions::scorer_setup);
+#ifdef VW_FEAT_LDA_ENABLED
   reductions.push_back(VW::reductions::lda_setup);
+#endif
   reductions.push_back(VW::reductions::cbzo_setup);
 
   // Reductions