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-28045][ML][PYTHON] add missing RankingEvaluator #24869

Closed
wants to merge 9 commits into from

Conversation

zhengruifeng
Copy link
Contributor

What changes were proposed in this pull request?

add missing RankingEvaluator

How was this patch tested?

added testsuites

@SparkQA
Copy link

SparkQA commented Jun 14, 2019

Test build #106497 has finished for PR 24869 at commit 896522f.

  • This patch fails Python style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 14, 2019

Test build #106498 has finished for PR 24869 at commit 883bd6f.

  • This patch fails Python style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 14, 2019

Test build #106500 has finished for PR 24869 at commit 28b1c57.

  • This patch fails Python style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 14, 2019

Test build #106503 has finished for PR 24869 at commit 94e3de6.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 14, 2019

Test build #106505 has finished for PR 24869 at commit 9f505c0.

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

@zhengruifeng
Copy link
Contributor Author

friendly ping @srowen

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Looks generally OK to me

setDefault(metricName -> "meanAveragePrecision")

final val k = new IntParam(this, "k",
"The ranking position value used in meanAveragePrecisionAtK|precisionAtK|ndcgAtK|recallAtK." +
Copy link
Member

Choose a reason for hiding this comment

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

Not a big deal, but could you generate this from the supportedMetrics by filtering those including "AtK"?

python/pyspark/ml/evaluation.py Show resolved Hide resolved
@SparkQA
Copy link

SparkQA commented Jun 20, 2019

Test build #106698 has finished for PR 24869 at commit f9b6777.

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

import org.apache.spark.mllib.util.MLlibTestSparkContext
import org.apache.spark.mllib.util.TestingUtils._


Copy link
Member

Choose a reason for hiding this comment

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

There are many extra blank lines below, but I wouldn't hold up merging it for that.

@SparkQA
Copy link

SparkQA commented Jun 25, 2019

Test build #106873 has finished for PR 24869 at commit 7de9bdf.

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

@srowen
Copy link
Member

srowen commented Jun 25, 2019

Merged to master

@srowen srowen closed this in c397b06 Jun 25, 2019
@zhengruifeng zhengruifeng deleted the ranking_eval branch June 26, 2019 01:07
kiku-jw pushed a commit to kiku-jw/spark that referenced this pull request Jun 26, 2019
## What changes were proposed in this pull request?
add missing RankingEvaluator

## How was this patch tested?
added testsuites

Closes apache#24869 from zhengruifeng/ranking_eval.

Authored-by: zhengruifeng <ruifengz@foxmail.com>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
@imatiach-msft
Copy link
Contributor

this is really great to see @zhengruifeng @srowen , can't wait to try this out

@ConstantinRuhdorfer
Copy link

This looks great! Sorry, maybe I am overlooking something but is this supposed to be used with the .transform() or .recommendForAllUsers() API in the case of ALS?

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