Merge pull request #1389 from eyalroz/fix-issue-1368

[REVIEW] refactored set_null_count()
rapidsai · Apr 13, 2019 · f6ad6de · f6ad6de
2 parents ea880d0 + 84c3d5b
commit f6ad6de
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 19 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,6 +30,7 @@
 - PR #1319 CSV Reader: Use column wrapper for gdf_column output alloc/dealloc
 - PR #1376 Change series quantile default to linear
 - PR #1399 Replace CFFI bindings for NVTX functions with Cython bindings
+- PR #1389 Refactored `set_null_count()`
 - PR #1386 Added macros `GDF_TRY()`, `CUDF_TRY()` and `ASSERT_CUDF_SUCCEEDED()`
 
 ## Bug Fixes

diff --git a/cpp/src/copying/gather.cu b/cpp/src/copying/gather.cu
@@ -332,8 +332,7 @@ struct column_gatherer {
                      gather_map, check_bounds, stream);
 
       // TODO compute the null count in the gather_bitmask kernels
-      gdf_error gdf_status = set_null_count(destination_column);
-      CUDF_EXPECTS(GDF_SUCCESS == gdf_status, "set_null_count failed");
+      set_null_count(*destination_column);
     }
 
     CHECK_STREAM(stream);

diff --git a/cpp/src/copying/scatter.cu b/cpp/src/copying/scatter.cu
@@ -165,8 +165,7 @@ struct column_scatterer {
                       scatter_map);
 
       // Update destination column's null count
-      gdf_error gdf_status = set_null_count(destination_column);
-      CUDF_EXPECTS(GDF_SUCCESS == gdf_status, "set_null_count failed");
+      set_null_count(*destination_column);
     }
 
     CHECK_STREAM(stream);

diff --git a/cpp/src/utilities/cudf_utils.h b/cpp/src/utilities/cudf_utils.h
@@ -7,6 +7,7 @@
 #include <cuda_runtime_api.h>
 
 #include <vector>
+#include <cassert>
 
 #ifdef __CUDACC__
 #define CUDA_HOST_DEVICE_CALLABLE __host__ __device__ inline
@@ -18,16 +19,19 @@
 #define CUDA_LAUNCHABLE
 #endif
 
-inline gdf_error set_null_count(gdf_column* col) {
-  gdf_size_type valid_count{};
-  gdf_error result =
-      gdf_count_nonzero_mask(col->valid, col->size, &valid_count);
-
-  GDF_REQUIRE(GDF_SUCCESS == result, result);
-
-  col->null_count = col->size - valid_count;
-
-  return GDF_SUCCESS;
+/**---------------------------------------------------------------------------*
+ * @brief Sets a @ref gdf_column 's (uninitialized) null_count value by
+ * counting the zeros in its validity indicator bits (if it has them).
+ **---------------------------------------------------------------------------*/
+inline void set_null_count(gdf_column& column) {
+    if (column.valid == nullptr) {
+        column.null_count = 0;
+    }
+    else {
+        gdf_size_type valid_count;
+        CUDF_TRY(gdf_count_nonzero_mask(column.valid, column.size, &valid_count));
+        column.null_count = column.size - valid_count;
+    }
 }
 
 /* --------------------------------------------------------------------------*/

diff --git a/cpp/tests/utilities/column_wrapper.cuh b/cpp/tests/utilities/column_wrapper.cuh
@@ -379,11 +379,7 @@ struct column_wrapper {
       the_column.valid = nullptr;
     }
 
-    gdf_error result = set_null_count(&the_column);
-    if (GDF_SUCCESS != result) {
-      throw std::runtime_error("Failed to set null count. Error code: " +
-                               std::to_string(result));
-    }
+    set_null_count(the_column);
   }
 
   rmm::device_vector<ColumnType> data;  ///< Container for the column's data