Fix all MSVC compilation issues (#250)

Fix all compilation issues with MSVC and also enable windows build in wheels and tests

Author: dudoslav

.github/workflows/build-wheels.yml

@@ -72,9 +72,7 @@ jobs:
           - [ ubuntu-22.04, manylinux_x86_64 ]
           - [ macos-13, macosx_x86_64 ]
           - [ macos-13, macosx_arm64 ]
-          # Windows build is currently disabled since the C++ build fails on multiple errors e.g. file manipulation
-          # and std::filesystem::path implicit to string conversion.
-          # Follow up task https://app.shortcut.com/tiledb-inc/story/41119/enable-windows-build-for-vector-search-repository
+          - [ windows-2022, win_amd64 ]
         python: [ "cp39", "cp310", "cp311", "cp312", "pp39" ]
         exclude:
           - buildplat: [ macos-13, macosx_arm64 ]

.github/workflows/ci.yml

@@ -10,15 +10,20 @@ jobs:
   run-tests:
     strategy:
       matrix:
-        os: [ubuntu-latest]
+        os: [ubuntu-latest, windows-latest]
     runs-on: ${{ matrix.os }}
     steps:
       - name: Install OpenBLAS
+        if: ${{ runner.os == 'Linux' }}
         run: sudo apt install libopenblas-dev
       - uses: actions/checkout@v3
       - name: Configure CMake
         run: cmake -S ./src -B ./src/build -DCMAKE_BUILD_TYPE=Debug -DTILEDB_VS_ENABLE_BLAS=ON
       - name: Build
         run: cmake --build ./src/build -j3
+      - name: Fix Windows dll inclusion with ctest
+        if: ${{ runner.os == 'Windows' }}
+        run: |
+            cp .\src\build\externals\install\bin\tiledb.dll D:/a/TileDB-Vector-Search/TileDB-Vector-Search/src/build/libtiledbvectorsearch/include/test
       - name: Run Tests
         run: cmake --build ./src/build --target check-ci

src/CMakeLists.txt

@@ -103,6 +103,11 @@ endif()
 # Control compiler-specific flags.
 include(CompilerFlags)
 
+if (MSVC)
+    add_compile_options(/bigobj)
+    add_compile_definitions("_ITERATOR_DEBUG_LEVEL=0")
+endif()
+
 if (NOT $ENV{CIBUILDWHEEL} EQUAL 1 AND NOT $ENV{CONDA_BUILD} EQUAL 1)
   if(MSVC)
       set(CMAKE_CXX_FLAGS_DEBUG "/Od /Zi /EHsc /RTC1" CACHE STRING "" FORCE)

src/cmake/Superbuild.cmake

@@ -150,9 +150,9 @@ add_custom_target(
 
 # make check
 add_custom_target(check
-  COMMAND ${CMAKE_CTEST_COMMAND} --test-dir ${CMAKE_CURRENT_BINARY_DIR}/libtiledbvectorsearch --output-on-failure
+  COMMAND ${CMAKE_CTEST_COMMAND} -C $<CONFIG> --test-dir ${CMAKE_CURRENT_BINARY_DIR}/libtiledbvectorsearch --output-on-failure
 )
 
 add_custom_target(check-ci
-  COMMAND ${CMAKE_CTEST_COMMAND} --test-dir ${CMAKE_CURRENT_BINARY_DIR}/libtiledbvectorsearch --output-on-failure -E \"unit_slicing|unit_ivf_index\"
+  COMMAND ${CMAKE_CTEST_COMMAND} -C $<CONFIG> --test-dir ${CMAKE_CURRENT_BINARY_DIR}/libtiledbvectorsearch --output-on-failure --extra-verbose -E \"unit_slicing|unit_ivf_index\"
 )

src/include/detail/linalg/tdb_partitioned_matrix.h

@@ -57,6 +57,7 @@
 #ifndef TILEDB_PARTITIONED_MATRIX_H
 #define TILEDB_PARTITIONED_MATRIX_H
 
+#include <algorithm>
 #include <cstddef>
 #include <future>
 #include <memory>
@@ -373,8 +374,8 @@ class tdbPartitionedMatrix
     }
 
     // auto max_resident_parts = 0UL;
-    auto running_resident_parts = 0UL;
-    auto running_resident_size = 0UL;
+    size_t running_resident_parts = 0UL;
+    size_t running_resident_size = 0UL;
     for (size_t i = 0; i < total_num_parts_; ++i) {
       auto part_size = master_indices_[relevant_parts_[i] + 1] -
                        master_indices_[relevant_parts_[i]];

src/include/index/index_defs.h

@@ -79,8 +79,7 @@ enum class IndexKind {
 [[maybe_unused]] static std::string current_storage_version{"0.3"};
 
 // @todo Use enum for key rather than string?
-using StorageFormat =
-    std::map<std::string, std::map<std::string, std::filesystem::path>>;
+using StorageFormat = std::map<std::string, std::map<std::string, std::string>>;
 [[maybe_unused]] static StorageFormat storage_formats = {
     {"0.1",
      {

src/include/index/index_group.h

@@ -76,7 +76,7 @@ class base_index_group {
 
  protected:
   std::reference_wrapper<const tiledb::Context> cached_ctx_;
-  std::filesystem::path group_uri_;
+  std::string group_uri_;
   size_t index_timestamp_{0};
   size_t group_timestamp_{0};
   size_t timetravel_index_{0};
@@ -279,12 +279,16 @@ class base_index_group {
   /** Convert an array name to a uri. */
   constexpr std::string array_name_to_uri(
       const std::string& array_name) const noexcept {
-    return group_uri_ / array_name;
+    return (std::filesystem::path{group_uri_} /
+            std::filesystem::path{array_name})
+        .string();
   }
 
   /** Convert an array key to a uri. */
   constexpr std::string array_key_to_uri(const std::string& array_key) const {
-    return group_uri_ / array_key_to_array_name(array_key);
+    return (std::filesystem::path{group_uri_} /
+            std::filesystem::path{array_key_to_array_name(array_key)})
+        .string();
   }
 
  public:
@@ -532,7 +536,7 @@ class base_index_group {
       std::cout << "# " + msg << std::endl;
     }
     std::cout << "-------------------------------------------------------\n";
-    std::cout << "Stored in " + group_uri_.string() + ":" << std::endl;
+    std::cout << "Stored in " + group_uri_ + ":" << std::endl;
     auto cfg = tiledb::Config();
     auto read_group = tiledb::Group(cached_ctx_, group_uri_, TILEDB_READ, cfg);
     for (size_t i = 0; i < read_group.member_count(); ++i) {

src/include/index/vamana_index.h

@@ -229,7 +229,7 @@ auto greedy_search(
         // @todo (actually it does, but shouldn't need it -- need to
         // investigate) if (result.template insert /*<unique_id>*/ (score, p)) {
         if (result.template insert<unique_id>(score, p)) {
-          q2.template insert /*<unique_id>*/ (score, p);
+          q2.insert(score, p);
         }
       }
     }