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

Added flatten and flatten2 BF16/FP32 FWD/BWD kernels #35892

Merged
merged 6 commits into from
Sep 27, 2021
Merged

Added flatten and flatten2 BF16/FP32 FWD/BWD kernels #35892

merged 6 commits into from
Sep 27, 2021

Conversation

jakpiase
Copy link
Contributor

@jakpiase jakpiase commented Sep 21, 2021 •
edited
Loading

PR types

New features

PR changes

OPs

Describe

Added(currently disabled) flatten and flatten2 BF16/FP32 FWD/BWD kernels

@paddle-bot-old
Copy link

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

@jczaja jczaja self-requested a review September 22, 2021 15:36
@jczaja
Copy link
Contributor

jczaja commented Sep 22, 2021

@tsocha Please review this PR

jczaja
jczaja approved these changes Sep 22, 2021
View reviewed changes
Copy link
Contributor

@jczaja jczaja left a comment

Choose a reason for hiding this comment

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

LGTM

@jakpiase
Copy link
Contributor Author

FYI:
Flatten, reshape and squeeze ops are all disabled and will be re-enabled when memory descriptor will be added to Tensor class. It was my intention to keep them disabled in this PR

lidanqing-intel
lidanqing-intel reviewed Sep 24, 2021
View reviewed changes
Copy link
Contributor

@lidanqing-intel lidanqing-intel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • If you commented the mkldnn kernels calling part, how does mkldnn kernels code coverage passed? It looks now the mkldnn UT will always only call native kernels, is it?
  • When will be memory descriptor being added into Tensor? What is the cause to add mem descriptor into Tensor?

@jakpiase
Copy link
Contributor Author

jakpiase commented Sep 24, 2021 •
edited
Loading

Yes, for now only native kernels are called in mkldnn UTs. I will be adding md into tensor at the beginning of Q4, so it is only a temporary solution. We need to add memory descriptor into tensor class, because it is impossible to support all kind of data layouts through the memory format tags.

lidanqing-intel
lidanqing-intel approved these changes Sep 25, 2021
View reviewed changes
Copy link
Contributor

@lidanqing-intel lidanqing-intel left a comment

Choose a reason for hiding this comment

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

LGTM

@jczaja jczaja merged commit e427a0f into PaddlePaddle:develop Sep 27, 2021
AnnaTrainingG pushed a commit to AnnaTrainingG/Paddle that referenced this pull request Sep 29, 2021
@jakpiase @AnnaTrainingG
Added flatten and flatten2 BF16/FP32 FWD/BWD kernels (PaddlePaddle#35892
963f3bf 
)

* refactored reshape multiop kernel and added flatten1/2 kernels

* added formatting for flatten tests

* CI fix

* disabled reshape_kernel ops after succesful CI run

* minor fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jczaja jczaja jczaja approved these changes

@lidanqing-intel lidanqing-intel lidanqing-intel approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@jakpiase @jczaja @lidanqing-intel