Skip to content

Conversation

tianbingsz
Copy link
Contributor

@tianbingsz tianbingsz commented Jan 4, 2017

Cosine Similarity Task for Paddle Function APIs (#977)

  1. add Cosine Similarity Forward function.
  2. add Cosine Similarity Backward function
  3. update forward/backward in CosSimLayer and CosSimVecMatLayer.
  4. clean unused code.
  5. merge daoyuan's FuncArg, address one of the comments.
  6. rewrite unit test using Daoyuan's new FunctionTest.

Copy link
Contributor

Choose a reason for hiding this comment

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

CosSimVecMatLayer.h还是跟CosSimVecMatLayer.cpp写在一起好,并没有另外一个文件需要include "CosSimVecMatLayer.h"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Test的写法还是比较复杂,对于只是Check CPU/GPU是否一致的test case还是得实现一个更简便的方法来做。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@tianbingsz
Copy link
Contributor Author

tianbingsz commented Jan 25, 2017

@hedaoyuan , 用新的FunctionTest重写了一下unit test, address 了你的comments.如果没有什么问题的话,可否approve一下(你新的两个op可以再开一个PR写一下)?

Copy link
Contributor

Choose a reason for hiding this comment

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

这个for (size_t i = 0; i < batchSize; i++)可以放到CosSimForwardFunc里面,同时Layer可以去掉这些tmp matrix。backward也一样可以去掉for (size_t i = 0; i < batchSize; i++)
CosSimForwardFunc区分是被CosSimVecMatLayer还是CosSimLayer调用应该是通过inputs[0],inputs[1]的参数类型上判断(shape不一样)。另外,也不建议在CosSimForwardFunc里面增加for (size_t i = 0; i < batchSize; i++),API应该可以直接支持。

@tianbingsz tianbingsz merged commit 8604666 into PaddlePaddle:develop Feb 8, 2017
wangxicoding pushed a commit to wangxicoding/Paddle that referenced this pull request Dec 9, 2021
* feat: add cnn_dailymail dataset

* fix: correct error message

* refactor: use paddlenlp decompress api

* docs: add doc for cnn_dm
lizexu123 pushed a commit to lizexu123/Paddle that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants