-
Notifications
You must be signed in to change notification settings - Fork 275
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
【Hackathon 5 No.16】为 Paddle 新增 EmbeddingBag API #687
Conversation
@luotao1 @Charles-hit 麻烦review下,谢谢。 |
|
||
# 四、对比分析 | ||
|
||
可以直接参考的实现是pytorch,涉及到的API在Paddle中均有实现,可以想到用Paddle API组合实现相同的逻辑 |
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.
这个任务需要在之前未开发完的PR上接着进行开发,需要手写embeddingbag算子。
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.
汗。可以在之前开发者https://github.com/PaddlePaddle/Paddle/pull/49000基础上进行开发。
这里写的是可以呀!
麻烦确认下是否能组和实现。
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.
出于性能的考量,这个算子要求写kernel的哈,因为之前已经开发过一版了,需要在那个pr基础上完成后续工作的开发。
|
||
## 命名与参数设计 | ||
|
||
API设计为`paddle.nn.functional.embedding_bag(num_embeddings, embedding_dim, padding_idx=None, sparse=False, weight_attr=None, name=None, mode='mean')`和`paddle.nn.EmbeddingBag(num_embeddings, embedding_dim, padding_idx=None, sparse=False, weight_attr=None, name=None, mode='mean')` |
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.
这儿forward函数签名需要也给出来
elif self._mode == "mean": | ||
return paddle.mean(out, axis=1) | ||
elif self._mode == "max": | ||
return paddle.max(out, axis=-1) |
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.
这儿axis是否应该是1
|
||
# 六、测试和验收的考量 | ||
|
||
测试考虑的case如下: |
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.
测试需要对sparse也进行测试,可以写一下完成的测试方案,比如考虑哪些边界条件。
@luotao1 @Charles-hit 麻烦确认下是否能组和实现? |
@lisamhy 如 #687 (comment) 所说,不能组合实现,需要新增 Kernel |
PaddlePaddle/Paddle#57262