Exclude tests from cov report & improve coverage (#119)

microsoft · Sep 13, 2023 · e4f47dd · e4f47dd
1 parent 3e540b0
commit e4f47dd
Show file tree

Hide file tree

Showing 24 changed files with 928 additions and 67 deletions.
diff --git a/.azure-pipelines/ut.yml b/.azure-pipelines/ut.yml
@@ -74,7 +74,7 @@ jobs:
       targetType: 'inline'
       script: |
         mkdir build && cd build
-        cmake -DCMAKE_BUILD_TYPE=Debug -DCODE_COVERAGE=ON ..
+        cmake -DCMAKE_BUILD_TYPE=Debug ..
         make -j ut
       workingDirectory: '$(System.DefaultWorkingDirectory)'
 
@@ -105,8 +105,15 @@ jobs:
       targetType: 'inline'
       script: |
         lcov --capture --directory . --output-file coverage.info
-        lcov --remove coverage.info '/usr/*' --output-file coverage.info
-        lcov --remove coverage.info '*/third_party/*' --output-file coverage.info
+        lcov --remove coverage.info \
+            '/usr/*' \
+            '/tmp/*' \
+            '*/third_party/*' \
+            '*/ark/*_test.*' \
+            '*/examples/*' \
+            '*/python/*' \
+            '*/ark/unittest/unittest_utils.cc' \
+            --output-file coverage.info
         lcov --list coverage.info
         bash <(curl -s https://codecov.io/bash) -f coverage.info || echo "Codecov did not collect coverage reports"
       workingDirectory: '$(System.DefaultWorkingDirectory)/build'

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -8,7 +8,6 @@ set(ARK_PATCH "1")
 set(ARK_VERSION "${ARK_MAJOR}.${ARK_MINOR}.${ARK_PATCH}")
 set(ARK_SOVERSION "${ARK_MAJOR}.${ARK_MINOR}")
 
-option(CODE_COVERAGE "Enable coverage reporting" OFF)
 option(USE_KAHYPAR "Use KaHyPar for scheduling" OFF)
 
 cmake_minimum_required(VERSION 3.25)
@@ -21,7 +20,7 @@ set(BUILD_DIR ${CMAKE_CURRENT_BINARY_DIR})
 
 # Code coverage from https://github.com/codecov/example-cpp11-cmake
 add_library(coverage_config INTERFACE)
-if(CODE_COVERAGE AND CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
+if(CMAKE_BUILD_TYPE MATCHES "Debug" AND CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
     # Add required flags (GCC & LLVM/Clang)
     target_compile_options(coverage_config INTERFACE
         -O0        # no optimization

diff --git a/ark/ark_test.cc b/ark/ark_test.cc
@@ -0,0 +1,25 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+#include "include/ark.h"
+#include "unittest/unittest_utils.h"
+
+ark::unittest::State test_version()
+{
+    auto version = ark::version();
+
+    // Check if the version string is in the correct format.
+    auto dot1 = version.find('.');
+    auto dot2 = version.find('.', dot1 + 1);
+    UNITTEST_NE(dot1, std::string::npos);
+    UNITTEST_NE(dot2, std::string::npos);
+
+    return ark::unittest::SUCCESS;
+}
+
+int main()
+{
+    ark::init();
+    UNITTEST(test_version);
+    return 0;
+}
diff --git a/ark/file_io.cc b/ark/file_io.cc
@@ -69,6 +69,14 @@ int create_dir(const string &path)
     return 0;
 }
 
+int remove_dir(const string &path)
+{
+    if (rmdir(path.c_str()) == -1) {
+        return errno;
+    }
+    return 0;
+}
+
 // Helper function to remove all files in a directory given a file descriptor.
 int clear_dirat_helper(int dir_fd, const char *name)
 {

diff --git a/ark/file_io.h b/ark/file_io.h
@@ -14,7 +14,8 @@ bool is_exist(const std::string &path);
 bool is_dir(const std::string &path);
 bool is_file(const std::string &path);
 int create_dir(const std::string &path);
-// int clear_dir(const std::string &path);
+int remove_dir(const std::string &path);
+int clear_dir(const std::string &path);
 std::vector<std::string> list_dir(const std::string &path);
 
 std::string read_file(const std::string &path);

diff --git a/ark/file_io_test.cc b/ark/file_io_test.cc
@@ -0,0 +1,125 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+#include "env.h"
+#include "file_io.h"
+#include "include/ark.h"
+#include "unittest/unittest_utils.h"
+#include <algorithm>
+#include <fstream>
+
+ark::unittest::State test_is_exist()
+{
+    UNITTEST_EQ(ark::is_exist("/"), true);
+    return ark::unittest::SUCCESS;
+}
+
+ark::unittest::State test_is_dir()
+{
+    UNITTEST_EQ(ark::is_dir("/"), true);
+    return ark::unittest::SUCCESS;
+}
+
+ark::unittest::State test_is_file()
+{
+    UNITTEST_EQ(ark::is_file("/"), false);
+    UNITTEST_EQ(ark::is_file(__FILE__), true);
+    return ark::unittest::SUCCESS;
+}
+
+ark::unittest::State test_create_remove_dir()
+{
+    std::string tmp_dir = ark::get_env().path_tmp_dir;
+    if (!ark::is_exist(tmp_dir)) {
+        UNITTEST_EQ(ark::create_dir(tmp_dir), 0);
+        UNITTEST_EQ(ark::is_exist(tmp_dir), true);
+        UNITTEST_EQ(ark::is_dir(tmp_dir), true);
+    }
+
+    auto test_dir = tmp_dir + "/test";
+    UNITTEST_EQ(ark::create_dir(test_dir), 0);
+    UNITTEST_EQ(ark::is_exist(test_dir), true);
+    UNITTEST_EQ(ark::is_dir(test_dir), true);
+
+    UNITTEST_EQ(ark::remove_dir(test_dir), 0);
+
+    return ark::unittest::SUCCESS;
+}
+
+ark::unittest::State test_list_get_clear_dir()
+{
+    std::string tmp_dir = ark::get_env().path_tmp_dir;
+    if (!ark::is_exist(tmp_dir)) {
+        UNITTEST_EQ(ark::create_dir(tmp_dir), 0);
+        UNITTEST_EQ(ark::is_exist(tmp_dir), true);
+        UNITTEST_EQ(ark::is_dir(tmp_dir), true);
+    }
+
+    auto test_file_1 = tmp_dir + "/test1.txt";
+    std::ofstream ofs(test_file_1);
+    ofs << "test";
+    ofs.close();
+
+    auto test_file_2 = tmp_dir + "/test2.txt";
+    ofs.open(test_file_2);
+    ofs << "test";
+    ofs.close();
+
+    UNITTEST_EQ(ark::is_exist(test_file_1), true);
+    UNITTEST_EQ(ark::is_file(test_file_1), true);
+
+    UNITTEST_EQ(ark::is_exist(test_file_2), true);
+    UNITTEST_EQ(ark::is_file(test_file_2), true);
+
+    auto files = ark::list_dir(tmp_dir);
+    UNITTEST_EQ(files.size(), 2UL);
+    std::sort(files.begin(), files.end());
+    UNITTEST_EQ(files[0], test_file_1);
+    UNITTEST_EQ(files[1], test_file_2);
+
+    UNITTEST_EQ(ark::get_dir(test_file_1), tmp_dir);
+    UNITTEST_EQ(ark::get_dir(test_file_2), tmp_dir);
+
+    UNITTEST_EQ(ark::clear_dir(tmp_dir), 0);
+
+    UNITTEST_EQ(ark::is_exist(test_file_1), false);
+    UNITTEST_EQ(ark::is_exist(test_file_2), false);
+
+    return ark::unittest::SUCCESS;
+}
+
+ark::unittest::State test_read_write_file()
+{
+    std::string tmp_dir = ark::get_env().path_tmp_dir;
+    if (!ark::is_exist(tmp_dir)) {
+        UNITTEST_EQ(ark::create_dir(tmp_dir), 0);
+        UNITTEST_EQ(ark::is_exist(tmp_dir), true);
+        UNITTEST_EQ(ark::is_dir(tmp_dir), true);
+    }
+
+    auto test_file = tmp_dir + "/test.txt";
+    std::string data = "test";
+    ark::write_file(test_file, data);
+    UNITTEST_EQ(ark::is_exist(test_file), true);
+    UNITTEST_EQ(ark::is_file(test_file), true);
+
+    auto read_data = ark::read_file(test_file);
+    UNITTEST_EQ(read_data, data);
+
+    UNITTEST_EQ(ark::remove_file(test_file), 0);
+    UNITTEST_EQ(ark::is_exist(test_file), false);
+
+    return ark::unittest::SUCCESS;
+}
+
+int main()
+{
+    ark::init();
+    UNITTEST(test_is_exist);
+    UNITTEST(test_is_dir);
+    UNITTEST(test_is_file);
+    UNITTEST(test_create_remove_dir);
+    UNITTEST(test_list_get_clear_dir);
+    UNITTEST(test_read_write_file);
+    return 0;
+}
diff --git a/ark/gpu/gpu_compile.cc b/ark/gpu/gpu_compile.cc
@@ -140,8 +140,7 @@ const string link(const vector<string> &ptxs)
 #endif // (ARK_USE_NVRTC)
 
 const string gpu_compile(const vector<string> &codes,
-                         const GpuArchType &arch_type, unsigned int max_reg_cnt,
-                         bool use_comm_sw)
+                         const GpuArchType &arch_type, unsigned int max_reg_cnt)
 {
     const string &ark_root = get_env().path_root_dir;
     string arch;
@@ -190,9 +189,7 @@ const string gpu_compile(const vector<string> &codes,
     }
     assert(items.size() == 1);
     para_exec<pair<string, string>>(
-        items, 20,
-        [&arch, &ark_root, max_reg_cnt,
-         use_comm_sw](pair<string, string> &item) {
+        items, 20, [&arch, &ark_root, max_reg_cnt](pair<string, string> &item) {
             string cu_file_path = item.second + ".cu";
             // Write CUDA code file.
             {
@@ -214,7 +211,7 @@ const string gpu_compile(const vector<string> &codes,
                          << "-I" << ark_root << "/include/kernels ";
             exec_cmd << "-ccbin g++ -std c++17 -lcuda "
                 "--define-macro=ARK_TARGET_CUDA_ARCH=" << arch << " "
-                "--define-macro=ARK_COMM_SW=" << (int)use_comm_sw << " " <<
+                "--define-macro=ARK_COMM_SW=1 " <<
                 include_args.str() <<
                 "-gencode arch=compute_" << arch
                 << ",code=sm_" << arch << " "

diff --git a/ark/gpu/gpu_compile.h b/ark/gpu/gpu_compile.h
@@ -12,8 +12,8 @@
 namespace ark {
 
 const std::string gpu_compile(const std::vector<std::string> &codes,
-                              const GpuArchType &arch, unsigned int max_reg_cnt,
-                              bool use_comm_sw);
+                              const GpuArchType &arch,
+                              unsigned int max_reg_cnt);
 
 } // namespace ark
 

diff --git a/ark/gpu/gpu_kernel.cc b/ark/gpu/gpu_kernel.cc
@@ -71,7 +71,7 @@ GpuKernel::~GpuKernel()
 }
 
 //
-void GpuKernel::compile(const GpuInfo &gpu_info, bool use_comm_sw)
+void GpuKernel::compile(const GpuInfo &gpu_info)
 {
     if (this->is_compiled()) {
         return;
@@ -83,8 +83,7 @@ void GpuKernel::compile(const GpuInfo &gpu_info, bool use_comm_sw)
     }
     //
     if (this->cubin.empty()) {
-        this->cubin =
-            gpu_compile(this->codes, gpu_info.arch, max_reg_cnt, use_comm_sw);
+        this->cubin = gpu_compile(this->codes, gpu_info.arch, max_reg_cnt);
     }
 
     //
@@ -178,8 +177,9 @@ GpuLoopKernel::GpuLoopKernel(const string &name_,
                 {},
                 {{0, sizeof(GpuPtr)}, {0, sizeof(GpuPtr)}},
                 cubin_},
-      ctx{ctx_}, timer_begin{ctx_->create_event(false, nullptr)},
-      timer_end{ctx_->create_event(false, nullptr)}
+      ctx{ctx_}, timer_begin{ctx_->create_event(false)}, timer_end{
+                                                             ctx_->create_event(
+                                                                 false)}
 {
     ctx_->set_current();
     this->flag = make_unique<GpuMem>(sizeof(int));
@@ -250,7 +250,7 @@ void GpuLoopKernel::compile(const GpuInfo &gpu_info)
         return;
     }
     // Compile the code.
-    GpuKernel::compile(gpu_info, this->ctx->is_comm_sw());
+    GpuKernel::compile(gpu_info);
 }
 
 void GpuLoopKernel::load()
@@ -293,33 +293,31 @@ void GpuLoopKernel::load()
             cuModuleGetGlobal(&clks_ptr_addr, 0, this->module, ARK_CLKS_NAME));
         CULOG(cuMemcpyHtoD(clks_ptr_addr, &clks_ptr_val, sizeof(GpuPtr)));
         // set the data buffer pointers of remote gpus
-        if (this->ctx->is_comm_sw()) {
-            int nrph = get_env().num_ranks_per_host;
-            int nodes_id = this->ctx->get_gpu_id() / nrph;
-            // only set the GPU remote data buf pointers of the GPUs on the same
-            // node
-            for (int i = nodes_id * nrph;
-                 i < (nodes_id + 1) * nrph && i < this->ctx->get_world_size();
-                 i++) {
-                GpuPtr data_buf_value = this->ctx->get_data_ref(i);
-                if (data_buf_value == 0) {
-                    continue;
-                }
-                GpuPtr data_buf_ptr;
-                string data_buf_name = ARK_BUF_NAME + std::to_string(i);
-                CUresult _e = cuModuleGetGlobal(&data_buf_ptr, 0, this->module,
-                                                data_buf_name.c_str());
-                // in some test code the symbol _ARK_BUF_0 is not defined
-                if (_e == CUDA_ERROR_NOT_FOUND) {
-                    LOG(DEBUG, "global variable ", data_buf_name, " not found");
-                    continue;
-                }
-                // CULOG(_e);
-                LOG(DEBUG, data_buf_name, " data_buf_ptr=", std::hex,
-                    data_buf_ptr, " data_buf_value=", data_buf_value);
-                CULOG(cuMemcpyHtoD(data_buf_ptr, &data_buf_value,
-                                   sizeof(GpuPtr)));
+
+        int nrph = get_env().num_ranks_per_host;
+        int nodes_id = this->ctx->get_gpu_id() / nrph;
+        // only set the GPU remote data buf pointers of the GPUs on the same
+        // node
+        for (int i = nodes_id * nrph;
+             i < (nodes_id + 1) * nrph && i < this->ctx->get_world_size();
+             i++) {
+            GpuPtr data_buf_value = this->ctx->get_data_ref(i);
+            if (data_buf_value == 0) {
+                continue;
+            }
+            GpuPtr data_buf_ptr;
+            string data_buf_name = ARK_BUF_NAME + std::to_string(i);
+            CUresult _e = cuModuleGetGlobal(&data_buf_ptr, 0, this->module,
+                                            data_buf_name.c_str());
+            // in some test code the symbol _ARK_BUF_0 is not defined
+            if (_e == CUDA_ERROR_NOT_FOUND) {
+                LOG(DEBUG, "global variable ", data_buf_name, " not found");
+                continue;
             }
+            // CULOG(_e);
+            LOG(DEBUG, data_buf_name, " data_buf_ptr=", std::hex, data_buf_ptr,
+                " data_buf_value=", data_buf_value);
+            CULOG(cuMemcpyHtoD(data_buf_ptr, &data_buf_value, sizeof(GpuPtr)));
         }
     }
 }

diff --git a/ark/gpu/gpu_kernel.h b/ark/gpu/gpu_kernel.h
@@ -32,7 +32,7 @@ class GpuKernel
               const std::string &cubin);
     ~GpuKernel();
 
-    void compile(const GpuInfo &gpu_info, bool use_comm_sw = true);
+    void compile(const GpuInfo &gpu_info);
     GpuState launch(GpuStream stream);
 
     const std::string &get_name()

diff --git a/ark/gpu/gpu_mgr.cc b/ark/gpu/gpu_mgr.cc
@@ -214,20 +214,14 @@ void GpuMgrCtx::destroy_stream(const GpuStream &s)
 }
 
 //
-GpuEvent GpuMgrCtx::create_event(bool disable_timing, CUipcEventHandle *handle)
+GpuEvent GpuMgrCtx::create_event(bool disable_timing)
 {
     GpuEvent cuda_event;
     unsigned int flags = 0;
     if (disable_timing) {
         flags |= CU_EVENT_DISABLE_TIMING;
     }
-    if (handle != nullptr) {
-        flags |= CU_EVENT_INTERPROCESS;
-    }
     CULOG(cuEventCreate(&cuda_event, flags));
-    if (handle != nullptr) {
-        CULOG(cuIpcGetEventHandle(handle, cuda_event));
-    }
     return cuda_event;
 }