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
【Hackathon No.19】Implement ASGD optimizer #42431
Closed
tiancaishaonvjituizi
wants to merge
12
commits into
PaddlePaddle:develop
from
tiancaishaonvjituizi:asgd_optimizer
Closed
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
ad8ceda
fix false positive warning in gcc>=9
tiancaishaonvjituizi 178dd27
use more aggressive way
tiancaishaonvjituizi 6cfd8ec
stash
tiancaishaonvjituizi c3b49d8
implement asgd optimizer
tiancaishaonvjituizi ef95457
refine
tiancaishaonvjituizi 2c6f29d
replace array_equal with allclose
tiancaishaonvjituizi 8a48bfb
add print to debug
tiancaishaonvjituizi 506d05b
add print to debug
tiancaishaonvjituizi 0403b08
add print to debug
tiancaishaonvjituizi f353873
update test
tiancaishaonvjituizi 3a21267
add print to debug
tiancaishaonvjituizi 46e5362
Merge branch 'develop' into asgd_optimizer
tiancaishaonvjituizi File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserved. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
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 "paddle/fluid/framework/infershape_utils.h" | ||
#include "paddle/fluid/framework/op_registry.h" | ||
#include "paddle/phi/core/infermeta_utils.h" | ||
#include "paddle/phi/infermeta/multiary.h" | ||
|
||
namespace paddle { | ||
namespace operators { | ||
|
||
using Tensor = framework::Tensor; | ||
|
||
class AsgdOp : public framework::OperatorWithKernel { | ||
public: | ||
using framework::OperatorWithKernel::OperatorWithKernel; | ||
|
||
framework::OpKernelType GetExpectedKernelType( | ||
const framework::ExecutionContext &ctx) const override { | ||
return framework::OpKernelType( | ||
OperatorWithKernel::IndicateVarDataType(ctx, "Param"), ctx.GetPlace()); | ||
} | ||
}; | ||
|
||
class AsgdOpMaker : public framework::OpProtoAndCheckerMaker { | ||
public: | ||
void Make() override { | ||
AddInput("Param", "(Tensor) Input parameter"); | ||
AddInput("Grad", "(Tensor) Input gradient"); | ||
AddInput("LearningRate", "(Tensor) Learning rate of SGD"); | ||
AddInput("AvgParam", | ||
"(Tensor) Average of parameter"); | ||
AddInput("CurrentStep", | ||
"(Tensor) Current step"); | ||
AddOutput("ParamOut", | ||
"(Tensor, same with Param) " | ||
"Output parameter, should share the same memory with Param"); | ||
AddOutput("AvgParamOut", | ||
"(Tensor, same with AvgParam) Average of parameter"); | ||
AddOutput("CurrentStepOut", | ||
"(Tensor) Increased step"); | ||
|
||
AddAttr<float>("t0", | ||
"(float, default 1e6) point at which to start averaging") | ||
.SetDefault(0.95f); | ||
AddComment(R"DOC( | ||
)DOC"); | ||
} | ||
}; | ||
|
||
} // namespace operators | ||
} // namespace paddle | ||
|
||
namespace ops = paddle::operators; | ||
DECLARE_INFER_SHAPE_FUNCTOR(asgd, AsgdInferMetaFunctor, | ||
PD_INFER_META(phi::AsgdInferMeta)); | ||
REGISTER_OPERATOR( | ||
asgd, ops::AsgdOp, ops::AsgdOpMaker, | ||
paddle::framework::EmptyGradOpMaker<paddle::framework::OpDesc>, | ||
paddle::framework::EmptyGradOpMaker<paddle::imperative::OpBase>, | ||
AsgdInferMetaFunctor); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// 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. | ||
|
||
#pragma once | ||
|
||
#include "paddle/phi/core/dense_tensor.h" | ||
|
||
namespace phi { | ||
|
||
template <typename T, typename Context> | ||
void AsgdKernel(const Context& dev_ctx, | ||
const DenseTensor& param, | ||
const DenseTensor& learning_rate, | ||
const DenseTensor& grad, | ||
const DenseTensor& avg_param, | ||
const DenseTensor& current_step, | ||
float t0, | ||
DenseTensor* param_out, | ||
DenseTensor* avg_param_out, | ||
DenseTensor* current_step_out); | ||
|
||
} // namespace phi |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
// Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// 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 "paddle/phi/kernels/asgd_kernel.h" | ||
|
||
#include "paddle/phi/backends/cpu/cpu_context.h" | ||
#include "paddle/phi/core/kernel_registry.h" | ||
#include "paddle/phi/kernels/funcs/eigen/common.h" | ||
#include "paddle/phi/kernels/funcs/eigen/eigen_function.h" | ||
|
||
namespace phi { | ||
|
||
template <typename T, typename Context> | ||
void AsgdKernel(const Context& dev_ctx, | ||
const DenseTensor& param, | ||
const DenseTensor& grad, | ||
const DenseTensor& learning_rate, | ||
const DenseTensor& avg_param, | ||
const DenseTensor& current_step, | ||
float t0, | ||
DenseTensor* param_out, | ||
DenseTensor* avg_param_out, | ||
DenseTensor* current_step_out) { | ||
dev_ctx.template Alloc<T>(param_out); | ||
dev_ctx.template Alloc<T>(avg_param_out); | ||
dev_ctx.template Alloc<T>(current_step_out); | ||
|
||
auto eigen_param = EigenVector<T>::Flatten(param); | ||
auto eigen_grad = EigenVector<T>::Flatten(grad); | ||
auto eigen_avg_param = EigenVector<T>::Flatten(avg_param); | ||
auto eigen_param_out = EigenVector<T>::Flatten(*param_out); | ||
auto eigen_avg_param_out = EigenVector<T>::Flatten(*avg_param_out); | ||
auto& place = *dev_ctx.eigen_device(); | ||
|
||
auto lr = learning_rate.data<T>()[0]; | ||
eigen_param_out.device(place) = eigen_param - lr * eigen_grad; | ||
|
||
T current_step_data = current_step.data<T>()[0]; | ||
|
||
if (current_step_data <= t0) { | ||
eigen_avg_param_out.device(place) = eigen_param_out; | ||
} else { | ||
const auto mu1 = 1 / (current_step_data - t0); | ||
const auto mu2 = 1 - mu1; | ||
eigen_avg_param_out.device(place) = | ||
mu2 * eigen_avg_param + mu1 * eigen_param_out; | ||
} | ||
*current_step_out->mutable_data<T>(dev_ctx.GetPlace()) = | ||
current_step_data + 1; | ||
} | ||
|
||
} // namespace phi | ||
|
||
PD_REGISTER_KERNEL(asgd, CPU, ALL_LAYOUT, phi::AsgdKernel, float, double) {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
// Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// 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 "paddle/phi/kernels/asgd_kernel.h" | ||
|
||
#include "paddle/phi/backends/gpu/gpu_context.h" | ||
#include "paddle/phi/core/kernel_registry.h" | ||
|
||
namespace phi { | ||
|
||
template <typename T> | ||
__global__ void ASGDKernel(const T* param, | ||
const T* grad, | ||
const T* learning_rate, | ||
const T* avg_param, | ||
const T* current_step, | ||
float t0, | ||
size_t num, | ||
T* param_out, | ||
T* avg_param_out) { | ||
T lr = learning_rate[0]; | ||
CUDA_KERNEL_LOOP(i, num) { param_out[i] = param[i] - lr * grad[i]; } | ||
T current_step_data = current_step[0]; | ||
if (current_step_data <= t0) { | ||
memcpy(avg_param_out, param, num * sizeof(T)); | ||
} else { | ||
const auto mu1 = 1 / (current_step_data - t0); | ||
const auto mu2 = 1 - mu1; | ||
CUDA_KERNEL_LOOP(i, num) { | ||
avg_param_out[i] = mu2 * avg_param[i] + mu1 * param_out[i]; | ||
} | ||
} | ||
} | ||
|
||
template <typename T> | ||
__global__ void IncreaseStep(const T* step, T* step_out) { | ||
*step_out = *step + 1; | ||
} | ||
|
||
template <typename T, typename Context> | ||
void AsgdKernel(const Context& dev_ctx, | ||
const DenseTensor& param, | ||
const DenseTensor& learning_rate, | ||
const DenseTensor& grad, | ||
const DenseTensor& avg_param, | ||
const DenseTensor& current_step, | ||
float t0, | ||
DenseTensor* param_out, | ||
DenseTensor* avg_param_out, | ||
DenseTensor* current_step_out) { | ||
int block = 512; | ||
int grid = (param.numel() + block - 1) / block; | ||
|
||
ASGDKernel<T><<<grid, block, 0, dev_ctx.stream()>>>( | ||
param.data<T>(), | ||
grad.data<T>(), | ||
learning_rate.data<T>(), | ||
avg_param.data<T>(), | ||
current_step.data<T>(), | ||
t0, | ||
param.numel(), | ||
param_out->mutable_data<T>(dev_ctx.GetPlace()), | ||
avg_param_out->mutable_data<T>(dev_ctx.GetPlace())); | ||
|
||
IncreaseStep<T><<<1, 1, 0, dev_ctx.stream()>>>( | ||
current_step.data<T>(), | ||
current_step_out->mutable_data<T>(dev_ctx.GetPlace())); | ||
} | ||
|
||
} // namespace phi | ||
|
||
PD_REGISTER_KERNEL(asgd, GPU, ALL_LAYOUT, phi::AsgdKernel, float, double) {} |
Binary file not shown.
Binary file not shown.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
【需要帮助】
这里有很奇怪的问题。这个测试在本地是可以通过的,但在 CI 环境里,第二次 loss.backward() 前后 model._w.grad 均为 None(通过这些 print 语句看出的)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI里提示有CONFLICT,先同步一下最新的代码再试试吧
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
conflict 是来自于我自己的这个 PR 被合并到 develop 了:#42265
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已同步
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个问题很奇怪。本地是完全正常的,但 ci 环境里 loss.backward() 后 grad 竟然是 None。
或者有什么方法强行设置 model._w 的 grad 也可以,我尝试过 model._w.grad = ... 但会报错,不知道有没有其他方式
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tiancaishaonvjituizi 您可太热心了,这个咱们之前讨论过了,感谢建议,我们后面会看下有没有更好的方式去替代这部分自动生成声明的实现,结合编译效率来看的话,每个文件中都展开一编注册的代码确实也不太高效,这确实不是一种好的方式。
不过其实这个设计在跟您在这儿讨论的这批人加入paddle之前就是这样的了,#14413
这个我理解在当时也起到了降低开发成本的作用,也没有您说的那么严重,但当然这并不是一种好的方式我也认可。我们后来人在此基础上的开发和重构都要在时效和目标之间做平衡和取舍,在目前的体系下,这样的方式既能够减少大家的开发成本,也能满足快速迭代上线的目标。如果我们先对整个paddle编译的体系进行重构优化的话,当然也能避免这种写法,但在算子库重构启动的时候这个不是最高优目标,这不只是一个技术的问题,涉及到团队规划和管理的多方面因素。
总之感谢您的建议,无论是编译方式和编译效率优化,还是Python API的分支写法优化,我们最近都在高优推进了,希望理解,也希望再给我们一些耐心,paddle会变得更好,也希望您能给我们更多的输入,大家一起将paddle建设得更好
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jzhang533 这和“使用宏的时候要小心”没有关系,可能是你还没有了解为什么这里换一下位置就会报错
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chenwhql 通俗的讲就是只想着赶紧做业务,忽视了对底层代码的维护和迭代,所以屎山代码越堆越大,直到某一天爆雷了,才意识到要花十倍的时间去弥补 😂 我可以理解,同时 “屎山代码越堆越大不是好事”也是不需要再专门说明原因的老生常谈。还是希望 paddle 团队多多自我审视,软件工程就是在和人“再苟一苟”的惰性斗争
这其实是一个常见的误区,原因是
其实这样做有两个方面的问题:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhiqiu
请问这一部分有后续的更新/详细计划吗?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
目前已经在开展部分工作,如#43247 ,将所有phi算子kernel编译为两个静态库。后续会逐步推广。