Fix CPU hist init for sparse dataset. (#4625)

* Fix CPU hist init for sparse dataset.

* Implement sparse histogram cut.
* Allow empty features.

* Fix windows build, don't use sparse in distributed environment.

* Comments.

* Smaller threshold.

* Fix windows omp.

* Fix msvc lambda capture.

* Fix MSVC macro.

* Fix MSVC initialization list.

* Fix MSVC initialization list x2.

* Preserve categorical feature behavior.

* Rename matrix to sparse cuts.
* Reuse UseGroup.
* Check for categorical data when adding cut.

Co-Authored-By: Philip Hyunsu Cho <chohyu01@cs.washington.edu>

* Sanity check.

* Fix comments.

* Fix comment.

tests/cpp/common/test_column_matrix.cc

@@ -7,6 +7,7 @@
 
 namespace xgboost {
 namespace common {
+
 TEST(DenseColumn, Test) {
   auto dmat = CreateDMatrix(100, 10, 0.0);
   GHistIndexMatrix gmat;
@@ -17,7 +18,7 @@ TEST(DenseColumn, Test) {
   for (auto i = 0ull; i < (*dmat)->Info().num_row_; i++) {
     for (auto j = 0ull; j < (*dmat)->Info().num_col_; j++) {
         auto col = column_matrix.GetColumn(j);
-        EXPECT_EQ(gmat.index[i * (*dmat)->Info().num_col_ + j],
+        ASSERT_EQ(gmat.index[i * (*dmat)->Info().num_col_ + j],
                   col.GetGlobalBinIdx(i));
     }
   }
@@ -33,7 +34,7 @@ TEST(SparseColumn, Test) {
   auto col = column_matrix.GetColumn(0);
   ASSERT_EQ(col.Size(), gmat.index.size());
   for (auto i = 0ull; i < col.Size(); i++) {
-    EXPECT_EQ(gmat.index[gmat.row_ptr[col.GetRowIdx(i)]],
+    ASSERT_EQ(gmat.index[gmat.row_ptr[col.GetRowIdx(i)]],
               col.GetGlobalBinIdx(i));
   }
   delete dmat;

tests/cpp/common/test_compressed_iterator.cc

@@ -28,7 +28,7 @@ TEST(CompressedIterator, Test) {
 
       CompressedIterator<int> ci(buffer.data(), alphabet_size);
       std::vector<int> output(input.size());
-      for (int i = 0; i < input.size(); i++) {
+      for (size_t i = 0; i < input.size(); i++) {
         output[i] = ci[i];
       }
 
@@ -38,12 +38,12 @@ TEST(CompressedIterator, Test) {
       std::vector<unsigned char> buffer2(
         CompressedBufferWriter::CalculateBufferSize(input.size(),
           alphabet_size));
-      for (int i = 0; i < input.size(); i++) {
+      for (size_t i = 0; i < input.size(); i++) {
         cbw.WriteSymbol(buffer2.data(), input[i], i);
       }
       CompressedIterator<int> ci2(buffer.data(), alphabet_size);
       std::vector<int> output2(input.size());
-      for (int i = 0; i < input.size(); i++) {
+      for (size_t i = 0; i < input.size(); i++) {
         output2[i] = ci2[i];
       }
       ASSERT_TRUE(input == output2);

tests/cpp/common/test_gpu_hist_util.cu

@@ -48,11 +48,11 @@ void TestDeviceSketch(const GPUSet& devices, bool use_external_memory) {
   int gpu_batch_nrows = 0;
 
   // find quantiles on the CPU
-  HistCutMatrix hmat_cpu;
-  hmat_cpu.Init((*dmat).get(), p.max_bin);
+  HistogramCuts hmat_cpu;
+  hmat_cpu.Build((*dmat).get(), p.max_bin);
 
   // find the cuts on the GPU
-  HistCutMatrix hmat_gpu;
+  HistogramCuts hmat_gpu;
   size_t row_stride = DeviceSketch(p, CreateEmptyGenericParam(0, devices.Size()), gpu_batch_nrows,
                                    dmat->get(), &hmat_gpu);
 
@@ -69,12 +69,12 @@ void TestDeviceSketch(const GPUSet& devices, bool use_external_memory) {
 
   // compare the cuts
   double eps = 1e-2;
-  ASSERT_EQ(hmat_gpu.min_val.size(), num_cols);
-  ASSERT_EQ(hmat_gpu.row_ptr.size(), num_cols + 1);
-  ASSERT_EQ(hmat_gpu.cut.size(), hmat_cpu.cut.size());
-  ASSERT_LT(fabs(hmat_cpu.min_val[0] - hmat_gpu.min_val[0]), eps * nrows);
-  for (int i = 0; i < hmat_gpu.cut.size(); ++i) {
-    ASSERT_LT(fabs(hmat_cpu.cut[i] - hmat_gpu.cut[i]), eps * nrows);
+  ASSERT_EQ(hmat_gpu.MinValues().size(), num_cols);
+  ASSERT_EQ(hmat_gpu.Ptrs().size(), num_cols + 1);
+  ASSERT_EQ(hmat_gpu.Values().size(), hmat_cpu.Values().size());
+  ASSERT_LT(fabs(hmat_cpu.MinValues()[0] - hmat_gpu.MinValues()[0]), eps * nrows);
+  for (int i = 0; i < hmat_gpu.Values().size(); ++i) {
+    ASSERT_LT(fabs(hmat_cpu.Values()[i] - hmat_gpu.Values()[i]), eps * nrows);
   }
 
   delete dmat;

tests/cpp/common/test_hist_util.cc

@@ -9,15 +9,7 @@
 namespace xgboost {
 namespace common {
 
-class HistCutMatrixMock : public HistCutMatrix {
- public:
-  size_t SearchGroupIndFromBaseRow(
-      std::vector<bst_uint> const& group_ptr, size_t const base_rowid) {
-    return HistCutMatrix::SearchGroupIndFromBaseRow(group_ptr, base_rowid);
-  }
-};
-
-TEST(HistCutMatrix, SearchGroupInd) {
+TEST(CutsBuilder, SearchGroupInd) {
   size_t constexpr kNumGroups = 4;
   size_t constexpr kNumRows = 17;
   size_t constexpr kNumCols = 15;
@@ -34,18 +26,102 @@ TEST(HistCutMatrix, SearchGroupInd) {
   p_mat->Info().SetInfo(
       "group", group.data(), DataType::kUInt32, kNumGroups);
 
-  HistCutMatrixMock hmat;
+  HistogramCuts hmat;
 
-  size_t group_ind = hmat.SearchGroupIndFromBaseRow(p_mat->Info().group_ptr_, 0);
+  size_t group_ind = CutsBuilder::SearchGroupIndFromRow(p_mat->Info().group_ptr_, 0);
   ASSERT_EQ(group_ind, 0);
 
-  group_ind = hmat.SearchGroupIndFromBaseRow(p_mat->Info().group_ptr_, 5);
+  group_ind = CutsBuilder::SearchGroupIndFromRow(p_mat->Info().group_ptr_, 5);
   ASSERT_EQ(group_ind, 2);
 
-  EXPECT_ANY_THROW(hmat.SearchGroupIndFromBaseRow(p_mat->Info().group_ptr_, 17));
+  EXPECT_ANY_THROW(CutsBuilder::SearchGroupIndFromRow(p_mat->Info().group_ptr_, 17));
 
   delete pp_mat;
 }
 
+namespace {
+class SparseCutsWrapper : public SparseCuts {
+ public:
+  std::vector<uint32_t> const& ColPtrs()   const { return p_cuts_->Ptrs(); }
+  std::vector<float>    const& ColValues() const { return p_cuts_->Values(); }
+};
+}  // anonymous namespace
+
+TEST(SparseCuts, SingleThreadedBuild) {
+  size_t constexpr kRows = 267;
+  size_t constexpr kCols = 31;
+  size_t constexpr kBins = 256;
+
+  // Dense matrix.
+  auto pp_mat = CreateDMatrix(kRows, kCols, 0);
+  DMatrix* p_fmat = (*pp_mat).get();
+
+  common::GHistIndexMatrix hmat;
+  hmat.Init(p_fmat, kBins);
+
+  HistogramCuts cuts;
+  SparseCuts indices(&cuts);
+  auto const& page = *(p_fmat->GetColumnBatches().begin());
+  indices.SingleThreadBuild(page, p_fmat->Info(), kBins, false, 0, page.Size(), 0);
+
+  ASSERT_EQ(hmat.cut.Ptrs().size(), cuts.Ptrs().size());
+  ASSERT_EQ(hmat.cut.Ptrs(), cuts.Ptrs());
+  ASSERT_EQ(hmat.cut.Values(), cuts.Values());
+  ASSERT_EQ(hmat.cut.MinValues(), cuts.MinValues());
+
+  delete pp_mat;
+}
+
+TEST(SparseCuts, MultiThreadedBuild) {
+  size_t constexpr kRows = 17;
+  size_t constexpr kCols = 15;
+  size_t constexpr kBins = 255;
+
+  omp_ulong ori_nthreads = omp_get_max_threads();
+  omp_set_num_threads(16);
+
+  auto Compare =
+#if defined(_MSC_VER)  // msvc fails to capture
+      [kBins](DMatrix* p_fmat) {
+#else
+      [](DMatrix* p_fmat) {
+#endif
+        HistogramCuts threaded_container;
+        SparseCuts threaded_indices(&threaded_container);
+        threaded_indices.Build(p_fmat, kBins);
+
+        HistogramCuts container;
+        SparseCuts indices(&container);
+        auto const& page = *(p_fmat->GetColumnBatches().begin());
+        indices.SingleThreadBuild(page, p_fmat->Info(), kBins, false, 0, page.Size(), 0);
+
+        ASSERT_EQ(container.Ptrs().size(), threaded_container.Ptrs().size());
+        ASSERT_EQ(container.Values().size(), threaded_container.Values().size());
+
+        for (uint32_t i = 0; i < container.Ptrs().size(); ++i) {
+          ASSERT_EQ(container.Ptrs()[i], threaded_container.Ptrs()[i]);
+        }
+        for (uint32_t i = 0; i < container.Values().size(); ++i) {
+          ASSERT_EQ(container.Values()[i], threaded_container.Values()[i]);
+        }
+      };
+
+  {
+    auto pp_mat = CreateDMatrix(kRows, kCols, 0);
+    DMatrix* p_fmat = (*pp_mat).get();
+    Compare(p_fmat);
+    delete pp_mat;
+  }
+
+  {
+    auto pp_mat = CreateDMatrix(kRows, kCols, 0.0001);
+    DMatrix* p_fmat = (*pp_mat).get();
+    Compare(p_fmat);
+    delete pp_mat;
+  }
+
+  omp_set_num_threads(ori_nthreads);
+}
+
 }  // namespace common
 }  // namespace xgboost

tests/cpp/common/test_random.cc

@@ -53,8 +53,8 @@ TEST(ColumnSampler, Test) {
 TEST(ColumnSampler, ThreadSynchronisation) {
   const int64_t num_threads = 100;
   int n = 128;
-  int iterations = 10;
-  int levels = 5;
+  size_t iterations = 10;
+  size_t levels = 5;
   std::vector<int> reference_result;
   bool success =
       true;  // Cannot use google test asserts in multithreaded region

tests/cpp/common/test_span.cc

@@ -310,7 +310,7 @@ TEST(Span, FirstLast) {
     ASSERT_EQ(first.size(), 4);
     ASSERT_EQ(first.data(), arr);
 
-    for (size_t i = 0; i < first.size(); ++i) {
+    for (int64_t i = 0; i < first.size(); ++i) {
       ASSERT_EQ(first[i], arr[i]);
     }
 
@@ -329,7 +329,7 @@ TEST(Span, FirstLast) {
     ASSERT_EQ(last.size(), 4);
     ASSERT_EQ(last.data(), arr + 12);
 
-    for (size_t i = 0; i < last.size(); ++i) {
+    for (int64_t i = 0; i < last.size(); ++i) {
       ASSERT_EQ(last[i], arr[i+12]);
     }
 
@@ -348,7 +348,7 @@ TEST(Span, FirstLast) {
     ASSERT_EQ(first.size(), 4);
     ASSERT_EQ(first.data(), s.data());
 
-    for (size_t i = 0; i < first.size(); ++i) {
+    for (int64_t i = 0; i < first.size(); ++i) {
       ASSERT_EQ(first[i], s[i]);
     }
 
@@ -368,7 +368,7 @@ TEST(Span, FirstLast) {
     ASSERT_EQ(last.size(), 4);
     ASSERT_EQ(last.data(), s.data() + 12);
 
-    for (size_t i = 0; i < last.size(); ++i) {
+    for (int64_t i = 0; i < last.size(); ++i) {
       ASSERT_EQ(s[12 + i], last[i]);
     }