Skip to content

Commit

Permalink
fix minor ASAN issues in UMAPAlgo::Optimize::find_params_ab() (#4405)
Browse files Browse the repository at this point in the history
There were actuall 2 minor issues that prevented `UMAPAlgo::Optimize::find_params_ab()` from being ASAN-clean at the moment:

- One is the mem leaks, of course
- Another one is the `malloc()`-`delete` mismatch -- only memory allocated using `new` or equivalent should be freed with operator `delete` or `delete[]`

Another issue that was also addressed here: exception safety (i.e., by using `make_unique` from C++-14)

Signed-off-by: Yitao Li <[email protected]>

Authors:
  - Yitao Li (https://github.com/yitao-li)

Approvers:
  - Zach Bjornson (https://github.com/zbjornson)
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #4405
  • Loading branch information
yitao-li authored Dec 6, 2021
1 parent c364d05 commit 5820133
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions cpp/src/umap/optimize.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,10 @@ void find_params_ab(UMAPParams* params, cudaStream_t stream)

float step = (spread * 3.0) / 300.0;

float* X = (float*)malloc(300 * sizeof(float));
float* y = (float*)malloc(300 * sizeof(float));
auto const X_uptr = std::make_unique<float[]>(300);
auto const y_uptr = std::make_unique<float[]>(300);
auto* const X = X_uptr.get();
auto* const y = y_uptr.get();

for (int i = 0; i < 300; i++) {
X[i] = i * step;
Expand All @@ -194,9 +196,10 @@ void find_params_ab(UMAPParams* params, cudaStream_t stream)

rmm::device_uvector<float> y_d(300, stream);
raft::update_device(y_d.data(), y, 300, stream);
float* coeffs_h = (float*)malloc(2 * sizeof(float));
coeffs_h[0] = 1.0;
coeffs_h[1] = 1.0;
auto const coeffs_h_uptr = std::make_unique<float[]>(2);
auto* const coeffs_h = coeffs_h_uptr.get();
coeffs_h[0] = 1.0;
coeffs_h[1] = 1.0;

rmm::device_uvector<float> coeffs(2, stream);
CUDA_CHECK(cudaMemsetAsync(coeffs.data(), 0, 2 * sizeof(float), stream));
Expand All @@ -211,8 +214,6 @@ void find_params_ab(UMAPParams* params, cudaStream_t stream)
CUDA_CHECK(cudaStreamSynchronize(stream));

CUML_LOG_DEBUG("a=%f, b=%f", params->a, params->b);

delete coeffs_h;
}
} // namespace Optimize
} // namespace UMAPAlgo

0 comments on commit 5820133

Please sign in to comment.