Skip to content
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

Cosine Similarity Paddle Function. #1061

Merged
merged 8 commits into from
Feb 8, 2017

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.

See the License for the specific language governing permissions and
limitations under the License. */

#pragma once
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.

See the License for the specific language governing permissions and
limitations under the License. */

#include <gtest/gtest.h>
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写一下)?

@@ -117,17 +127,25 @@ void CosSimVecMatLayer::forward(PassType passType) {
}

MatrixPtr outV = getOutputValue();

CHECK(outV && inV0 && inV1);
REGISTER_TIMER_INFO("FwCosVMTimer", getName().c_str());
for (size_t i = 0; i < batchSize; i++) {
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