-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-11171][SPARK-11237][SPARK-11241][ML] Try adding PMMLExportable to ML with KMeans #9207
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
[SPARK-11171][SPARK-11237][SPARK-11241][ML] Try adding PMMLExportable to ML with KMeans #9207
Conversation
|
Test build #44104 has finished for PR 9207 at commit
|
368879f to
1749aec
Compare
|
Test build #44105 has finished for PR 9207 at commit
|
|
Test build #44111 has finished for PR 9207 at commit
|
… only (since actual PMML model evaluation is to be done through a spark-packages project and previous JIRA decided re-loading PMML is out of project scope)
|
cc @jkbradley |
|
Test build #44241 has finished for PR 9207 at commit
|
|
re-ping @jkbradley |
1 similar comment
|
re-ping @jkbradley |
|
Also maybe @dbtsai if you have a chance to look at this that would be great. |
|
So I've updated this against master @jkbradley |
|
Test build #46990 has finished for PR 9207 at commit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems this is a copy-paste of org.apache.spark.mllib.pmml, should we deprecate the mllib one, and use the new one in ml package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that might be good, the main difference is this avoids using the factory implementation that the MLLib API was.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Uses the same public facing API as per the JIRA discussion re: lack of complaints from users with old API)
|
ping @jkbradley |
|
Test build #48499 has finished for PR 9207 at commit
|
|
Test build #49179 has finished for PR 9207 at commit
|
|
jenkins retest this please |
|
Test build #49214 has finished for PR 9207 at commit
|
|
Test build #49819 has finished for PR 9207 at commit
|
|
Jenkins retest this please |
|
Test build #49837 has finished for PR 9207 at commit
|
|
Test build #61683 has finished for PR 9207 at commit
|
|
Test build #61684 has finished for PR 9207 at commit
|
|
Test build #61784 has finished for PR 9207 at commit
|
|
HiveSubmit failures seem unrelated, jenkins retest this please. |
|
Test build #61799 has finished for PR 9207 at commit
|
|
Test build #62478 has finished for PR 9207 at commit
|
|
Now that 2.0 RC5 has passed - maybe it would be an OK time to revisit this? I think it can also be an ok base for adding more formats if we need to. |
|
Test build #63086 has finished for PR 9207 at commit
|
|
Test build #63193 has finished for PR 9207 at commit
|
|
Test build #63295 has finished for PR 9207 at commit
|
|
ping @MLnick now that 2.0 is out for awhile. |
|
Test build #65123 has finished for PR 9207 at commit
|
|
Ping @jkbradley / @MLnick - we are coming up on a year since we told people we would add PMML export to Spark in about a year so I figured I'd try and update this again. I'll update this PR tonight against the latest master. |
|
Test build #68724 has finished for PR 9207 at commit
|
|
@MLnick: Do you have the bandwith to revisit this? I'm open to refactoring to a more plug-gable approach if we've got the review bandwidth for it. |
|
Test build #82102 has finished for PR 9207 at commit
|
|
Closing this in favor of the approach taken in #19876 |
Add PMMLExportable to ML & use it for KMeans in ML pipeline. This is a bit different than the PMMLExportable in MLLib since the default trait doesn't go through a factory but instead depends on the model implementing it itself (and at the start that will mostly just be calling the parent model's method).
Still a WIP but wanted to see if anyone had comments about the slight change.