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

[SPARK-25321][ML] Fix local LDA model constructor #22510

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@WeichenXu123
Copy link
Contributor

commented Sep 21, 2018

What changes were proposed in this pull request?

change back the constructor to:

class LocalLDAModel private[ml] (
    uid: String,
    vocabSize: Int,
    private[clustering] val oldLocalModel : OldLocalLDAModel,
    sparkSession: SparkSession)

Although it is marked private[ml], it is used in mleap and the master change breaks mleap building.
See mleap code here

How was this patch tested?

Manual.

@SparkQA

This comment has been minimized.

Copy link

commented Sep 21, 2018

Test build #96393 has finished for PR 22510 at commit 2b2fdaf.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

asfgit pushed a commit that referenced this pull request Sep 21, 2018

[SPARK-25321][ML] Fix local LDA model constructor
## What changes were proposed in this pull request?

change back the constructor to:
```
class LocalLDAModel private[ml] (
    uid: String,
    vocabSize: Int,
    private[clustering] val oldLocalModel : OldLocalLDAModel,
    sparkSession: SparkSession)
```

Although it is marked `private[ml]`, it is used in `mleap` and the master change breaks `mleap` building.
See mleap code [here](https://github.com/combust/mleap/blob/c7860af328d519cf56441b4a7cd8e6ec9d9fee59/mleap-spark/src/main/scala/org/apache/spark/ml/bundle/ops/clustering/LDAModelOp.scala#L57)
## How was this patch tested?

Manual.

Closes #22510 from WeichenXu123/LDA_fix.

Authored-by: WeichenXu <weichen.xu@databricks.com>
Signed-off-by: Xiangrui Meng <meng@databricks.com>
(cherry picked from commit 40edab2)
Signed-off-by: Xiangrui Meng <meng@databricks.com>
@mengxr

This comment has been minimized.

Copy link
Contributor

commented Sep 21, 2018

LGTM. Merged into master and branch 2.4. Thanks for checking compatibility with MLeap.

@asfgit asfgit closed this in 40edab2 Sep 21, 2018

@WeichenXu123 WeichenXu123 deleted the WeichenXu123:LDA_fix branch Apr 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.