-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Pnp evaluator #5108
Pnp evaluator #5108
Conversation
…h original implementation
…h original implementation
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.
没啥大问题,提了一点小建议。
std::make_pair(query[i], std::vector<PredictionResult>())); | ||
} | ||
predictions[query[i]].push_back(PredictionResult( | ||
score[i * width + column], label[i], has_weight ? weight[i] : 1.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.
这里是不是也可以用emplace
?
predictions[query[i]].emplace_back(score[i * width + column], label[i], has_weight ? weight[i] : 1.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.
Done.
auto query = query_t->data<int32_t>(); | ||
const T* weight = nullptr; | ||
auto has_weight = weight_t != nullptr; | ||
if (has_weight) { |
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.
Why not if (weight_t != nullptr)
and remove line 52 ?
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.
Done.
|
||
auto score = score_t->data<T>(); | ||
auto label = label_t->data<T>(); | ||
auto query = query_t->data<int32_t>(); |
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.
我们要不要考虑query id
有可能不是数字的情况?
即使我们只支持数字的话,这里用size_t
是不是更合适?
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.
- 我似乎没看到我们有支持string类型tensor的design doc,所以我假定这里是原来的实现一样,只支持整数的;而且由于这里事实上是ID,所以用数字也是比较符合直觉的。之后如果有其它需要我们可以再扩展。
- 我参考了下其它要用到int类型input的op(lookup_table_op, top_k_op), 这里用
int32_t
确实和它们用int64_t
不一致,所以我把这里改为int64_t
了。size_t
是unsigned,和这里语义有区别。
# group by query id | ||
predictions = {} | ||
batch_size = label.shape[0] | ||
print "batch_size=", batch_size |
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.
Remove debug code?
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.
Done.
self.check_output() | ||
|
||
|
||
class TestPositiveNegativePairOpAccumulateWeight(OpTest): |
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.
TestPositiveNegativePairOpAccumulate
和TestPositiveNegativePairOpAccumulateWeight
有比较多的重复code, 可以考虑合并下。
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.
Done
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.
LGTM
This evaluator counts positive pairs, negative pairs in the ranking result.
Tested on the 1-dimension score case.
resolve #5086