-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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-39446][MLLIB] Add relevance score for nDCG evaluation #36843
Closed
+120
−45
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
631f91a
Add relevance to nDCG evaluation
uchiiii 74ac0a5
Format by ./dev/scalafmt
uchiiii c78a4e2
Remove the third input from (Int, Double) to Double
uchiiii de7b922
Modify conditions
uchiiii 075aa42
Revert formating map code
uchiiii 2ef33d5
Change infix style to dot style
uchiiii 3902ec8
Revert formating map code (remove indent)
uchiiii 95733af
Revert formating map code
uchiiii 6b3dc8b
Add since tags
uchiiii d653135
Update docs
uchiiii a172172
Update docs
uchiiii File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Oh, we need to update the
@Since
tags. I think you can somehow add a@Since
annotation to the default constructor args? as it's since 3.4.0. I'm not sure exactly where it goes. The old constructor can remain since 1.2.0. If that doesn't work, maybe we can leave this constructor and add the new one as a newdef this(...)
since 3.4.0?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 made changes and updated the docs.
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 we should refer to
BinaryClassificationMetrics
andMulticlassMetrics
, in whichRDD[_ <: Product]
was used as the input.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.
since end users now mainly use the
.ml
, is there any plan to expose this function to.ml
?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.
Sorry in advance for my poor understanding. I have some questions.
.ml
? Do you meanorg.apache.spark.ml
?RDD[_ <: Product]
is used to makeMulticlassMetric
class available to.ml
?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.
For the first one - yeah this change is in the 'older' .mllib package. I don't think there is an equivalent for it in the DataFrame-based .ml packages, so, maybe we can ignore that here. But if nDCG is supported in the .ml package somewhere and I forgot it, would be good to add it there too.
The declaration suggested here might actually work for both input types without a separate constructor. Try it maybe? if it works, yes, that is simpler, and lets this API support even more inputs
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.
Thank you for your explanation!
Could you review this?
#36920