Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LogisticRegressionMG][FEA] Support training when dataset contains only one class #5655

Merged
merged 5 commits into from
Nov 29, 2023

Conversation

lijinf2
Copy link
Contributor

@lijinf2 lijinf2 commented Nov 14, 2023

This pull request introduces functionality for C++ training on datasets with a single label. It helps Spark Rapids ML match Spark's behavior. Additionally, it updates the Dask class to generate an error message, consistent with Scikit-learn's behavior.

This PR depends on #5632

@lijinf2 lijinf2 requested review from a team as code owners November 14, 2023 18:52
Copy link

copy-pr-bot bot commented Nov 14, 2023

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions bot added Cython / Python Cython or Python issue CUDA/C++ labels Nov 14, 2023
@lijinf2 lijinf2 added non-breaking Non-breaking change 3 - Ready for Review Ready for review by team labels Nov 14, 2023
@lijinf2 lijinf2 force-pushed the fea_lrmg_onelabel branch 2 times, most recently from 2adfc37 to 7d5c635 Compare November 14, 2023 18:56
@lijinf2 lijinf2 added the improvement Improvement / enhancement to an existing function label Nov 14, 2023
@csadorf
Copy link
Contributor

csadorf commented Nov 14, 2023

/ok to test

python/cuml/linear_model/logistic_regression_mg.pyx Outdated Show resolved Hide resolved
python/cuml/linear_model/logistic_regression_mg.pyx Outdated Show resolved Hide resolved
python/cuml/tests/dask/test_dask_logistic_regression.py Outdated Show resolved Hide resolved
python/cuml/tests/dask/test_dask_logistic_regression.py Outdated Show resolved Hide resolved
Comment on lines 400 to 401
if not isinstance(cu_preds, np.ndarray):
cu_preds = cu_preds.to_numpy()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we expect there? Sparse arrays?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cu_preds stores predicted labels in a dense array or cudf.

python/cuml/tests/dask/test_dask_logistic_regression.py Outdated Show resolved Hide resolved
python/cuml/tests/dask/test_dask_logistic_regression.py Outdated Show resolved Hide resolved
python/cuml/linear_model/logistic_regression_mg.pyx Outdated Show resolved Hide resolved
python/cuml/linear_model/base_mg.pyx Outdated Show resolved Hide resolved
@lijinf2 lijinf2 force-pushed the fea_lrmg_onelabel branch 2 times, most recently from 2cc76f5 to ce40be2 Compare November 28, 2023 19:49
Copy link
Contributor

@csadorf csadorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor suggestions, but in principle no objections. LGTM!

python/cuml/tests/dask/test_dask_logistic_regression.py Outdated Show resolved Hide resolved
python/cuml/tests/dask/test_dask_logistic_regression.py Outdated Show resolved Hide resolved
python/cuml/tests/dask/test_dask_logistic_regression.py Outdated Show resolved Hide resolved
@cjnolet
Copy link
Member

cjnolet commented Nov 29, 2023

/merge

@rapids-bot rapids-bot bot merged commit 97b6fa3 into rapidsai:branch-23.12 Nov 29, 2023
49 checks passed
@lijinf2 lijinf2 deleted the fea_lrmg_onelabel branch June 26, 2024 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team CUDA/C++ Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants