[SPARK-11712] [ML] Make spark.ml LDAModel be abstract #9678
Conversation
There was a problem hiding this comment.
Should we warn that this (any any method that uses this) will trigger a collect in DistributedLDAModel (if so, does the doc belong here or in DistributedLDAModel), or can we expect users of DistributedLDAModel understand that already?
There was a problem hiding this comment.
This does not trigger a collect. It's oldLocalModel which can trigger a collect.
I'll add some more warnings in places where it can happen in the public API.
|
LGTM, all comments minor / optional |
|
Test build #2051 has finished for PR 9678 at commit
|
|
Test build #45774 has finished for PR 9678 at commit
|
|
@feynmanliang Thanks for reviewing! I think I addressed everything. |
|
Test build #45785 has finished for PR 9678 at commit
|
|
Merging with master, branch-1.6 |
Per discussion in the initial Pipelines LDA PR [#9513], we should make LDAModel abstract and create a LocalLDAModel. This code simplification should be done before the 1.6 release to ensure API compatibility in future releases. CC feynmanliang mengxr Author: Joseph K. Bradley <[email protected]> Closes #9678 from jkbradley/lda-pipelines-2. (cherry picked from commit dcb896f) Signed-off-by: Joseph K. Bradley <[email protected]>
Per discussion in the initial Pipelines LDA PR [apache#9513], we should make LDAModel abstract and create a LocalLDAModel. This code simplification should be done before the 1.6 release to ensure API compatibility in future releases. CC feynmanliang mengxr Author: Joseph K. Bradley <[email protected]> Closes apache#9678 from jkbradley/lda-pipelines-2.
3 participants
Per discussion in the initial Pipelines LDA PR [https://github.com//pull/9513], we should make LDAModel abstract and create a LocalLDAModel. This code simplification should be done before the 1.6 release to ensure API compatibility in future releases.
CC @feynmanliang @mengxr