Skip to content

Conversation

@ronny1996
Copy link
Contributor

PR types

Others

PR changes

OPs

Describe

add momentum_op_npu and test

@paddle-bot-old
Copy link

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

@ronny1996 ronny1996 force-pushed the npu_momentum branch 2 times, most recently from bf39afe to d9e32ee Compare July 12, 2021 11:04
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License. */
#include <string>
Copy link
Contributor

Choose a reason for hiding this comment

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

#include 这个头文件不需要?

from test_momentum_op import calculate_momentum_by_numpy

paddle.enable_static()
SEED = 2021
Copy link
Contributor

Choose a reason for hiding this comment

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

SEED没有用到?

self.shape = (123, 321)
self.use_nesterov = True


Copy link
Contributor

Choose a reason for hiding this comment

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

这个单测文件最好也是参考 test_momentum_op.py 把里面所有的单测case尽量都加上,保证测试的coverage。

Copy link
Contributor Author

@ronny1996 ronny1996 Jul 15, 2021

Choose a reason for hiding this comment

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

已添加其他case,NPU没有SparseMomentum相关算子,SparseMomentum相关单测未添加

qili93
qili93 previously approved these changes Jul 16, 2021
Copy link
Contributor

@qili93 qili93 left a comment

Choose a reason for hiding this comment

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

LGTM

@ronny1996 ronny1996 changed the title add momentum_op_npu and test [NPU] add momentum_op_npu and test Jul 28, 2021
Comment on lines +2156 to +2160
auto &pool = platform::DeviceContextPool::Instance();
auto devices = platform::GetSelectedNPUDevices();
for (size_t i = 0; i < devices.size(); ++i) {
pool.Get(platform::NPUPlace(devices[i]))->Wait();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add wait here?

Copy link
Contributor

@zhiqiu zhiqiu left a comment

Choose a reason for hiding this comment

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

LGTM for ShareDataWith

Copy link
Contributor

@qili93 qili93 left a comment

Choose a reason for hiding this comment

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

LGTM

@qili93 qili93 merged commit 9e3e08f into PaddlePaddle:develop Aug 11, 2021
@ronny1996 ronny1996 deleted the npu_momentum branch September 9, 2021 07:52
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.

3 participants