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.

Sign up for GitHub

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

Jump to bottom

Add SyncBatchNorm #26032

Merged
merged 12 commits into from
Aug 19, 2020
Merged

Add SyncBatchNorm #26032

merged 12 commits into from
Aug 19, 2020

Conversation

ceci3
Copy link
Contributor

@ceci3 ceci3 commented Aug 7, 2020 •
edited
Loading

PR types

Others

PR changes

APIs

Describe

Add SyncBatchNorm API for dygraph

  1. because moving mean and moving variance computed all the time in the batch norm C++ kernel , so 'track_running_stats' is not kick in the API now. need to change batch norm kernel.
  2. in the train mode, use mini-batch stats, in the eval mode, use global stats, not use use_global_stas and trainable_stats to jugde whether to use global or mini-batch stats

@ceci3 ceci3 force-pushed the add_syncbn branch 2 times, most recently from 892bbda to 808572f Compare August 7, 2020 05:50
@ceci3 ceci3 closed this Aug 7, 2020
@ceci3 ceci3 reopened this Aug 7, 2020
@ceci3 ceci3 force-pushed the add_syncbn branch 5 times, most recently from 2a24b15 to 62eb999 Compare August 9, 2020 11:50
zhiqiu
zhiqiu reviewed Aug 10, 2020
View reviewed changes
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 op_function_generator.cc

wanghaoshuang
wanghaoshuang reviewed Aug 10, 2020
View reviewed changes
python/paddle/fluid/dygraph/nn.py Outdated Show resolved Hide resolved
python/paddle/fluid/dygraph/nn.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/parallel_dygraph_sync_batch_norm.py Outdated
@@ -0,0 +1,90 @@
# Copyright (c) 2018 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright (c) 2020

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, thanks

python/paddle/fluid/tests/unittests/test_parallel_dygraph_sync_batch_norm.py Outdated Show resolved Hide resolved
swtkiwi
swtkiwi reviewed Aug 10, 2020
View reviewed changes
python/paddle/fluid/dygraph/nn.py Outdated
with fluid.dygraph.guard():
x = to_variable(x)
if fluid.is_compiled_with_cuda():
sync_batch_norm = nn.SyncBatchNorm(10)
Copy link
Contributor

Choose a reason for hiding this comment

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

辛苦用注释的形式给出输入、输出示例。如果PR着急，辛苦merge后再提一个PR进行更新。

@zhiqiu zhiqiu self-requested a review August 11, 2020 06:23
zhiqiu
zhiqiu previously approved these changes Aug 11, 2020
View reviewed changes
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 op_function_generator.cc

@ceci3
Copy link
Contributor Author

ceci3 commented Aug 11, 2020

image

swtkiwi
swtkiwi reviewed Aug 11, 2020
View reviewed changes
python/paddle/fluid/dygraph/nn.py Outdated
import numpy as np

x = np.array([[[[0.3, 0.4], [0.3, 0.07]], [[0.83, 0.37], [0.18, 0.93]]]]).astype('float32')
with fluid.dygraph.guard():
Copy link
Contributor

Choose a reason for hiding this comment

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

示例可以直接用paddle2.0-alpha的api
with paddle.imperative.guard():

zhiqiu
zhiqiu previously approved these changes Aug 17, 2020
View reviewed changes
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 op_function_generator.cc

@ceci3 ceci3 requested a review from willthefrog August 17, 2020 05:59
willthefrog
willthefrog previously approved these changes Aug 18, 2020
View reviewed changes
Copy link
Contributor

@willthefrog willthefrog left a comment

Choose a reason for hiding this comment

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

need subsequent change for converting batchnorms, otherwise LGTM

@ceci3 ceci3 requested a review from willthefrog August 19, 2020 02:40
willthefrog
willthefrog approved these changes Aug 19, 2020
View reviewed changes
Copy link
Contributor

@willthefrog willthefrog left a comment

Choose a reason for hiding this comment

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

LGTM

@zhiqiu zhiqiu self-requested a review August 19, 2020 03:27
zhiqiu
zhiqiu approved these changes Aug 19, 2020
View reviewed changes
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

@ceci3 ceci3 merged commit 56890dc into PaddlePaddle:develop Aug 19, 2020
@ceci3 ceci3 deleted the add_syncbn branch August 19, 2020 03:34
@ceci3 ceci3 mentioned this pull request Aug 21, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swtkiwi swtkiwi swtkiwi left review comments

@zhiqiu zhiqiu zhiqiu approved these changes

@willthefrog willthefrog willthefrog approved these changes

@qingqing01 qingqing01 Awaiting requested review from qingqing01

@wanghaoshuang wanghaoshuang Awaiting requested review from wanghaoshuang

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ceci3 @willthefrog @zhiqiu @wanghaoshuang @swtkiwi