Skip to content

Conversation

phlrain
Copy link
Collaborator

@phlrain phlrain commented Jan 29, 2022

PR types

Breaking changes

PR changes

OPs

Describe

move norm to pten

@paddle-bot-old
Copy link

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

// See the License for the specific language governing permissions and
// limitations under the License.

#include "paddle/fluid/operators/eigen/eigen_function.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

现在pten下的eigen应该可以用了

// limitations under the License.

#include "paddle/pten/kernels/norm_kernel.h"
#include "paddle/fluid/operators/eigen/eigen_function.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

namespace cub = hipcub;
#endif
#include "paddle/fluid/operators/amp/fp16_type_traits.h"
#include "paddle/fluid/platform/bfloat16.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

bfloat16也迁到pten了

namespace cub = hipcub;
#endif
#include "paddle/fluid/operators/amp/fp16_type_traits.h"
#include "paddle/fluid/platform/bfloat16.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

}
}

framework::KernelSignature GetExpectedPtenKernelArgs(
Copy link
Contributor

Choose a reason for hiding this comment

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

前向的这个映射函数建议就在pten/ops/compat目录下维护吧

#include "paddle/pten/core/ddim.h"

namespace pten {
inline void GetDims(
Copy link
Contributor

Choose a reason for hiding this comment

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

impl目录下定位是放置CPU和GPU代码公用的kernel实现,如果只是辅助函数,建议将函数放到pten/funcs,比如可以放到pten/funcs/common_shape.h中

// 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

是不是需要include一下norm_grad_kernel.h

// 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

同上

// 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

建议include一下norm_kernel.h



if __name__ == '__main__':
import paddle
Copy link
Contributor

Choose a reason for hiding this comment

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

import paddle应该可以移到文件头部

Copy link
Contributor

@ceci3 ceci3 left a comment

Choose a reason for hiding this comment

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

LGTM

@phlrain phlrain merged commit ece200b into PaddlePaddle:develop Feb 8, 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.

4 participants