Skip to content

Conversation

@fengxiaoshuai
Copy link
Contributor

PR types

Others

PR changes

Others

Describe

jetson-op-test

@paddle-bot-old
Copy link

paddle-bot-old bot commented Sep 1, 2021

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

XieYunshen
XieYunshen previously approved these changes Sep 2, 2021
Copy link
Contributor

@XieYunshen XieYunshen left a comment

Choose a reason for hiding this comment

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

LGTM
由于机器原因,jetson上的单测执行时间较长

XieYunshen
XieYunshen previously approved these changes Sep 3, 2021
#ifdef __HIPCC__
// HIP will throw core dump when threads > 256
constexpr int num_threads = 256;
#elif WITH_NV_JETSON
Copy link
Member

Choose a reason for hiding this comment

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

这个有测试过么? __HIPCC__应该是为支持华为硬件的宏,NVIDA的gpu不会运行到这个分支(WITH_GPU的时候不会编译HIP代码)。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个有测试过么? __HIPCC__应该是为支持华为硬件的宏,NVIDA的gpu不会运行到这个分支(WITH_GPU的时候不会编译HIP代码)。

经确认逻辑正确

dim3 threads(1024, 1);
int thread_num = 1024;
#ifdef WITH_NV_JETSON
// platform::ChangeThreadNum(context, &thread_num);
Copy link
Member

Choose a reason for hiding this comment

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

此处为何没有保持风格一致,也使用函数调用?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

此处为何没有保持风格一致,也使用函数调用?

这个op比较特殊,在调用的时候不需要传入设备上下文,所以无法根据运行时的实际设备调整线程数,只能在编译的时候指定,如果修改op的参数定义的话,涉及到的调用位置都要补充获取设备上下文参数的逻辑,影响面比较大。这样简单的修改话会影响agx和nx, 不过我让王也测试过,之前的模型性能没有下降,反而略微提升,所以目前看来可以这样指定。

Copy link
Member

@shangzhizhou shangzhizhou left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Superjomn Superjomn left a comment

Choose a reason for hiding this comment

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

LGTM

@shangzhizhou shangzhizhou merged commit c4a3e8b into PaddlePaddle:develop Sep 8, 2021
2742195759 pushed a commit to 2742195759/Paddle that referenced this pull request Sep 10, 2021
* merge CMakeList.txt manual

* add platform for changethreadnum

* repair some bugs according to make error

* do nothing just flush CI

* forget change thread num

* add inplace_atol param for check_output_with_place

* Windows

* std:min and std::max should be change because of windows
AnnaTrainingG pushed a commit to AnnaTrainingG/Paddle that referenced this pull request Sep 29, 2021
* merge CMakeList.txt manual

* add platform for changethreadnum

* repair some bugs according to make error

* do nothing just flush CI

* forget change thread num

* add inplace_atol param for check_output_with_place

* Windows

* std:min and std::max should be change because of windows
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.

4 participants