Skip to content

fix CatBoostRanker score computation #2231

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

fix CatBoostRanker score computation #2231

wants to merge 2 commits into from

Conversation

dmitrySorokin
Copy link

fix for #2230

@dmitrySorokin dmitrySorokin mentioned this pull request Nov 21, 2022
Closed
Evgueni-Petrov-aka-espetrov
Evgueni-Petrov-aka-espetrov requested changes Nov 21, 2022
View reviewed changes
catboost/python-package/catboost/core.py
@@ -6228,7 +6228,7 @@ def get_ndcg_metric_name(values, names):
raise CatBoostError("group_id must be initialized. If groups are not expected, pass an array of zeros")

predictions = self.predict(X)
return _eval_metric_util([y], [predictions], get_ndcg_metric_name(), None, group_id, group_weight, None, None, thread_count)[0]
return _eval_metric_util([y], [predictions], get_ndcg_metric_name([top, type, denominator], ['top', 'type', 'denominator']), None, group_id, group_weight, None, None, thread_count)[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

please also fix the condition in get_ndcg_metric_name

if np.all(np.array(values) == None):
    ...

Copy link
Author

Choose a reason for hiding this comment

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

Done

@Evgueni-Petrov-aka-espetrov
Copy link
Contributor

@dmitrySorokin could you please read and sign the Yandex CLA so that we could merge this PR?
https://catboost.ai/en/docs/concepts/development-and-contributions#yandex-cla

@dmitrySorokin
Copy link
Author

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=ru

@Evgueni-Petrov-aka-espetrov
Copy link
Contributor

@arcadia-devtools Ship it!

@arcadia-devtools
Copy link
Collaborator

CLA already signed

@Evgueni-Petrov-aka-espetrov
Copy link
Contributor

@arcadia-devtools Ship it!

@arcadia-devtools
Copy link
Collaborator

@Evgueni-Petrov-aka-espetrov, internal review request created: 3181666

@Evgueni-Petrov-aka-espetrov
Copy link
Contributor

@dmitrySorokin
the internal PR has been merged, and will appear on github soon
many thanks for fixing this bug 🔥 🔥 🔥

arcadia-devtools added a commit that referenced this pull request Nov 23, 2022
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
ea09fef 
…from #2231

MERGED FROM #2231

ref:45d16edecc536ec082e8142dc5b3d6f37dcb0c67
arcadia-devtools added a commit that referenced this pull request Nov 30, 2022
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
2a8b6ce 
…from #2231

MERGED FROM #2231

ref:0ae1fbb03db9f2665671c675f5bf7e5c2f548cc7
arcadia-devtools added a commit that referenced this pull request Dec 4, 2022
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
08972fa 
…from #2231

MERGED FROM #2231

ref:944110b731c75f2e1356394f2cdf1ed581d28322
robot-piglet pushed a commit that referenced this pull request Dec 27, 2022
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
3ff62f3 
…from #2231

MERGED FROM #2231

ref:02a6e62230e432a414fab9c00afbd6ff05b2468f
robot-piglet pushed a commit that referenced this pull request Jan 3, 2023
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
08e6563 
…from #2231

MERGED FROM #2231

ref:7e9961023effab7da9e1a84d90a289eb4d947140
robot-piglet pushed a commit that referenced this pull request Jan 5, 2023
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
d569d25 
…from #2231

MERGED FROM #2231

ref:609b01952a6b9ba09e497fbe1199b70c19cbf630
robot-piglet pushed a commit that referenced this pull request Jan 17, 2023
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
22e67b7 
…from #2231

MERGED FROM #2231

ref:45d16edecc536ec082e8142dc5b3d6f37dcb0c67
robot-piglet pushed a commit that referenced this pull request Jan 17, 2023
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
6df9344 
…from #2231

MERGED FROM #2231

ref:0ae1fbb03db9f2665671c675f5bf7e5c2f548cc7
robot-piglet pushed a commit that referenced this pull request Jan 17, 2023
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
7b9017d 
…from #2231

MERGED FROM #2231

ref:944110b731c75f2e1356394f2cdf1ed581d28322
robot-piglet pushed a commit that referenced this pull request Jan 17, 2023
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
dd85539 
…from #2231

MERGED FROM #2231

ref:02a6e62230e432a414fab9c00afbd6ff05b2468f
robot-piglet pushed a commit that referenced this pull request Jan 17, 2023
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
b695d93 
…from #2231

MERGED FROM #2231

ref:7e9961023effab7da9e1a84d90a289eb4d947140
robot-piglet pushed a commit that referenced this pull request Jan 17, 2023
@arcadia-devtools
Pull request "fix CatBoostRanker score computation" by @dmitrySorokin …
2a3101f 
…from #2231

MERGED FROM #2231

ref:609b01952a6b9ba09e497fbe1199b70c19cbf630
@andrey-khropov
Copy link
Member

This PR has been merged in ea09fef

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Evgueni-Petrov-aka-espetrov Evgueni-Petrov-aka-espetrov Evgueni-Petrov-aka-espetrov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dmitrySorokin @Evgueni-Petrov-aka-espetrov @arcadia-devtools @andrey-khropov