Skip to content

Commit ebb8e80

Browse files
authored
A few small fixes for multi-gpu PCA/TruncatedSVD (#7560)
- Don't branch the sign flipping behavior based on the version of sklearn installed. This somehow slipped through in #7331. We always want `cuml` behavior to be the same regardless of sklearn version - the only thing we branch on is the testing where we don't assert sign matches for sklearn < 1.5 (this matches the single-gpu testing strategy as well). - Adds a sync point in multi-gpu PCA before calling `signFlipComponents`. The multi-gpu implementation makes use of multiple streams, but before only the first stream was passed to `signFlipComponents` (without any sync beforehand) leading to potential stream ordering issues. It's hard to prove a negative, but with this change I can no longer reproduce an issue reported in `rapids_singlecell`. A similar fix isn't needed for `TruncatedSVD` since that implementation only uses one stream. Authors: - Jim Crist-Harif (https://github.com/jcrist) Approvers: - Simon Adorf (https://github.com/csadorf) URL: #7560
1 parent d24b272 commit ebb8e80

File tree

5 files changed

+22
-10
lines changed

5 files changed

+22
-10
lines changed

cpp/src/pca/pca_mg.cu

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ void fit_impl(raft::handle_t& handle,
7878
n_streams,
7979
true);
8080
} else {
81+
for (std::uint32_t i = 0; i < n_streams; i++) {
82+
handle.sync_stream(streams[i]);
83+
}
8184
signFlipComponents(handle,
8285
input_data[0]->ptr,
8386
components,
@@ -191,6 +194,9 @@ void fit_impl(raft::handle_t& handle,
191194
n_streams,
192195
true);
193196
} else {
197+
for (std::uint32_t i = 0; i < n_streams; i++) {
198+
handle.sync_stream(streams[i]);
199+
}
194200
signFlipComponents(h,
195201
input_data[0]->ptr,
196202
vMatrix.data(),

python/cuml/cuml/decomposition/pca_mg.pyx

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,7 @@
22
# SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION.
33
# SPDX-License-Identifier: Apache-2.0
44
#
5-
65
import numpy as np
7-
import sklearn
8-
from packaging.version import Version
96

107
import cuml.internals
118
from cuml.decomposition import PCA
@@ -94,7 +91,7 @@ class PCAMG(BaseDecompositionMG, PCA):
9491
cdef uintptr_t noise_variance_ptr = noise_variance.ptr
9592
cdef bool use_float32 = (dtype == np.float32)
9693
cdef handle_t* handle_ = <handle_t*><size_t>self.handle.getHandle()
97-
cdef bool flip_signs_based_on_U = (Version(sklearn.__version__) < Version("1.5.0"))
94+
cdef bool flip_signs_based_on_U = self._u_based_sign_flip
9895

9996
# Perform fit
10097
with nogil:

python/cuml/cuml/decomposition/tsvd_mg.pyx

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,7 @@
22
# SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION.
33
# SPDX-License-Identifier: Apache-2.0
44
#
5-
65
import numpy as np
7-
import sklearn
8-
from packaging.version import Version
96

107
import cuml.internals
118
from cuml.decomposition import TruncatedSVD
@@ -100,7 +97,7 @@ class TSVDMG(BaseDecompositionMG, TruncatedSVD):
10097
cdef uintptr_t singular_values_ptr = singular_values.ptr
10198
cdef bool use_float32 = dtype == np.float32
10299
cdef handle_t* handle_ = <handle_t*><size_t>self.handle.getHandle()
103-
cdef bool flip_signs_based_on_U = (Version(sklearn.__version__) < Version("1.5.0"))
100+
cdef bool flip_signs_based_on_U = self._u_based_sign_flip
104101

105102
# Perform Fit
106103
with nogil:

python/cuml/tests/dask/test_dask_pca.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
# SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION.
22
# SPDX-License-Identifier: Apache-2.0
33
#
4-
54
import cupy as cp
65
import numpy as np
76
import pytest
7+
import sklearn
8+
from packaging.version import Version
89

910
from cuml.dask.common.dask_arr_utils import to_dask_cudf
1011

12+
SKLEARN_GE_1_5_0 = Version(sklearn.__version__) >= Version("1.5.0")
13+
1114

1215
@pytest.mark.mg
1316
@pytest.mark.parametrize("nrows", [1000])
@@ -56,11 +59,12 @@ def test_pca_fit(nrows, ncols, n_parts, input_type, client):
5659
]
5760

5861
for attr in all_attr:
62+
with_sign = SKLEARN_GE_1_5_0 if attr == "components_" else True
5963
cuml_res = getattr(cupca, attr)
6064
if type(cuml_res) is np.ndarray:
6165
cuml_res = cuml_res.to_numpy()
6266
skl_res = getattr(skpca, attr)
63-
assert array_equal(cuml_res, skl_res, 1e-1, with_sign=True)
67+
assert array_equal(cuml_res, skl_res, 1e-1, with_sign=with_sign)
6468

6569

6670
@pytest.mark.mg

python/cuml/tests/dask/test_dask_tsvd.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,14 @@
55
import cupy as cp
66
import numpy as np
77
import pytest
8+
import sklearn
9+
from packaging.version import Version
810

911
from cuml.dask.common.dask_arr_utils import to_dask_cudf
1012
from cuml.testing.utils import array_equal, stress_param, unit_param
1113

14+
SKLEARN_GE_1_5_0 = Version(sklearn.__version__) >= Version("1.5.0")
15+
1216

1317
@pytest.mark.mg
1418
@pytest.mark.parametrize(
@@ -73,6 +77,10 @@ def test_tsvd_fit(data_info, input_type, client):
7377
skl_res = getattr(sktsvd, attr)
7478
if attr == "singular_values_":
7579
assert array_equal(cuml_res, skl_res, 1, with_sign=True)
80+
elif attr == "components_":
81+
assert array_equal(
82+
cuml_res, skl_res, 1e-1, with_sign=SKLEARN_GE_1_5_0
83+
)
7684
else:
7785
assert array_equal(cuml_res, skl_res, 1e-1, with_sign=True)
7886

0 commit comments

Comments
 (0)