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.

Sign up for GitHub

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

Jump to bottom

Ridge Regression support in oneapi #2743

Merged
merged 18 commits into from
May 21, 2024
Merged

Ridge Regression support in oneapi #2743

merged 18 commits into from
May 21, 2024

Conversation

DDJHB
Copy link
Contributor

@DDJHB DDJHB commented Apr 24, 2024 •
edited

Description

Changes made:

  1. Added GPU implementation for ridge regression as extension of linear regression
  2. Added CPU switch to daal implementation of ridge regression
  3. Added tests for GPU changes
  4. This PR's scope includes alpha parameter as a single value

@DDJHB DDJHB changed the title Add alpha parameter to Linear Regression to support Ridge Ridge Regression support in oneapi May 2, 2024
@DDJHB
Copy link
Contributor Author

DDJHB commented May 8, 2024

/intelci: run

@DDJHB
Copy link
Contributor Author

DDJHB commented May 10, 2024

/intelci: run

@DDJHB
Copy link
Contributor Author

DDJHB commented May 10, 2024

/intelci: run

@DDJHB
Copy link
Contributor Author

DDJHB commented May 13, 2024

/intelci: run

Alexandr-Solovev
Alexandr-Solovev reviewed May 13, 2024
View reviewed changes
Copy link
Contributor

@Alexandr-Solovev Alexandr-Solovev left a comment

Choose a reason for hiding this comment

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

In general LGTM with minor comments. Good work! Wait the CI checks and also, please compare the performance between online kernel vs batch.

cpp/oneapi/dal/algo/linear_regression/backend/gpu/train_kernel_norm_eq_dpc.cpp Outdated Show resolved Hide resolved
cpp/oneapi/dal/algo/linear_regression/common.cpp Show resolved Hide resolved
cpp/oneapi/dal/algo/linear_regression/test/fixture.hpp Outdated Show resolved Hide resolved
@DDJHB
Copy link
Contributor Author

DDJHB commented May 14, 2024

/intelci: run

@DDJHB
Copy link
Contributor Author

DDJHB commented May 14, 2024

/intelci: run

@DDJHB
Copy link
Contributor Author

DDJHB commented May 15, 2024

/intelci: run

@DDJHB
Copy link
Contributor Author

DDJHB commented May 15, 2024

/intelci: run

@DDJHB DDJHB marked this pull request as ready for review May 15, 2024 11:36
Vika-F
Vika-F reviewed May 16, 2024
View reviewed changes
Copy link
Contributor

@Vika-F Vika-F left a comment

Choose a reason for hiding this comment

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

Thank you for adding Ridge Regression support into oneDAL.
The code look good to me in general, but I think some parts could look better.
Please see the comments below.

cpp/oneapi/dal/algo/linear_regression/backend/cpu/finalize_train_kernel_norm_eq.cpp Outdated Show resolved Hide resolved
cpp/oneapi/dal/algo/linear_regression/backend/cpu/train_kernel_norm_eq.cpp Outdated Show resolved Hide resolved
cpp/oneapi/dal/algo/linear_regression/backend/cpu/train_kernel_norm_eq.cpp Show resolved Hide resolved
cpp/oneapi/dal/algo/linear_regression/common.hpp Show resolved Hide resolved
cpp/oneapi/dal/algo/linear_regression/test/fixture.hpp Outdated Show resolved Hide resolved
cpp/oneapi/dal/algo/linear_regression/backend/gpu/misc.hpp Outdated Show resolved Hide resolved
@DDJHB
Copy link
Contributor Author

DDJHB commented May 16, 2024

/intelci: run

@DDJHB
Copy link
Contributor Author

DDJHB commented May 17, 2024

/intelci: run

@DDJHB
Copy link
Contributor Author

DDJHB commented May 17, 2024

/intelci: run

Alexandr-Solovev
Alexandr-Solovev approved these changes May 21, 2024
View reviewed changes
Copy link
Contributor

@Alexandr-Solovev Alexandr-Solovev left a comment

Choose a reason for hiding this comment

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

LGTM! Good job, please update the copyright and doublecheck private ci

@DDJHB
Copy link
Contributor Author

DDJHB commented May 21, 2024

/intelci: run

@DDJHB DDJHB merged commit d6f4dc3 into oneapi-src:main May 21, 2024
16 of 17 checks passed
@icfaust icfaust mentioned this pull request May 28, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Vika-F Vika-F Vika-F left review comments

@Alexandr-Solovev Alexandr-Solovev Alexandr-Solovev approved these changes

@icfaust icfaust Awaiting requested review from icfaust

@Alexsandruss Alexsandruss Awaiting requested review from Alexsandruss Alexsandruss is a code owner

@samir-nasibli samir-nasibli Awaiting requested review from samir-nasibli samir-nasibli is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@DDJHB @Vika-F @Alexandr-Solovev