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

Add TensorExpression #239

Merged
merged 11 commits into from
Dec 6, 2016
Merged

Add TensorExpression #239

merged 11 commits into from
Dec 6, 2016

Conversation

hedaoyuan
Copy link
Contributor

A new TensorExpression implementation. And there is a documentation , to assist in code review. This document is not complete, at present, do not consider merger into the master.

@reyoung reyoung changed the base branch from master to develop October 26, 2016 08:12
@byzhang
Copy link

byzhang commented Oct 29, 2016

I LOVE this PR.

Copy link
Collaborator

@reyoung reyoung left a comment

Choose a reason for hiding this comment

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

添加文档

@@ -117,6 +125,9 @@ endif(NOT WITH_TIMER)
if(WITH_AVX)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${AVX_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${AVX_FLAGS}")
if(AVX_FOUND)
Copy link
Contributor

Choose a reason for hiding this comment

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

change to WITH_GPU

@byzhang
Copy link

byzhang commented Oct 31, 2016

Just FYI, https://github.com/ddemidov/vexcl

Thanks,
-B

On Sun, Oct 30, 2016 at 8:51 PM, gangliao [email protected] wrote:

@gangliao commented on this pull request.

In CMakeLists.txt
#239 (review):

@@ -117,6 +125,9 @@ endif(NOT WITH_TIMER)
if(WITH_AVX)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${AVX_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${AVX_FLAGS}")

  • if(AVX_FOUND)

change to WITH_GPU


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#239 (review), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AA1O1X2rkkpss7Yd78Glo3K3fzDTvqtUks5q5WWogaJpZM4Ke62g
.

@reyoung
Copy link
Collaborator

reyoung commented Nov 2, 2016

抱歉,之前看的时候没注意到有文档。

@backyes
Copy link
Contributor

backyes commented Nov 5, 2016

@hedaoyuan @reyoung 这个PR先不要merge吧,为了确保下一个milestone的release能比较stable,这个PR升级比较多,可以考虑放到下下个milestone,你们觉得呢?
另外, 因为GPU编译未使能的avx的bug https://github.com/baidu/Paddle/issues/282, 是否可以单独fix? @hedaoyuan

@reyoung
Copy link
Collaborator

reyoung commented Nov 7, 2016

@hedaoyuan says This document is not complete, at present, do not consider merger into the master.

@hedaoyuan
Copy link
Contributor Author

Merge最新的develop代码,并且对tests中的一些conflicts进行修改。

Copy link
Collaborator

@reyoung reyoung left a comment

Choose a reason for hiding this comment

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

LGTM

TensorGpuApply<T>(*this, expr);
} else {
TensorCpuApply<T>(*this, expr);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

虽然不是特别严重的事情,不过 operator =似乎应该返回自身的引用。这样才能写a=b=c。不过我们也可以说,不能这么写code。

@reyoung reyoung merged commit 82774db into PaddlePaddle:develop Dec 6, 2016
gglin001 pushed a commit to graphcore/Paddle-fork that referenced this pull request Dec 8, 2021
AnnaTrainingG pushed a commit to AnnaTrainingG/Paddle that referenced this pull request Sep 19, 2022
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.

7 participants