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

Make spark-rapids-ml support ml connect plugin #835

Open
wants to merge 34 commits into
base: branch-25.04
Choose a base branch
from

Conversation

wbo4958
Copy link
Collaborator

@wbo4958 wbo4958 commented Feb 10, 2025

@wbo4958 wbo4958 changed the title [POC] Make spark-rapids-ml support ml plugin Make spark-rapids-ml support ml connect plugin Feb 20, 2025
@wbo4958 wbo4958 marked this pull request as ready for review February 20, 2025 02:32
Copy link
Collaborator

@eordentlich eordentlich left a comment

Choose a reason for hiding this comment

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

Nice work! A few comments.

Copy link
Collaborator

@eordentlich eordentlich left a comment

Choose a reason for hiding this comment

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

one more comment to clarify how to build.

@rishic3
Copy link
Collaborator

rishic3 commented Feb 24, 2025

Curious about our project structure. We have a top-level jvm folder — does it make sense to have this java folder and the scala code together as subdirectories there?

@wbo4958 wbo4958 requested a review from eordentlich February 28, 2025 06:56
Copy link
Collaborator

@eordentlich eordentlich left a comment

Choose a reason for hiding this comment

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

Looks like you addressed all comments. Nice!
We should probably put this into branch-25.04, for next release. Can you redirect PR?

@rishic3
Copy link
Collaborator

rishic3 commented Mar 4, 2025

As a follow-up we should remove the Scala code example from the top-level README

@wbo4958 wbo4958 changed the base branch from branch-25.02 to branch-25.04 March 4, 2025 08:57
*
* @param uid unique ID of the estimator
*/
class RapidsLogisticRegression(override val uid: String) extends LogisticRegression with RapidsEstimator {
Copy link
Collaborator Author

@wbo4958 wbo4958 Mar 5, 2025

Choose a reason for hiding this comment

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

Hi @eordentlich, How do you think of the naming of RapidsLogisticRegression?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants