Skip to content

[Paddle] Add nn layer - #361

Merged
ksivaman merged 6 commits into
NVIDIA:mainfrom
Wong4j:jaywan/add_nn_layer
Aug 17, 2023
Merged

[Paddle] Add nn layer#361
ksivaman merged 6 commits into
NVIDIA:mainfrom
Wong4j:jaywan/add_nn_layer

Conversation

@Wong4j

@Wong4jWong4j commented Aug 6, 2023

Copy link
Copy Markdown
Contributor

Add the following nn layers (FP16/BF16):

  • FusedScaleMaskSoftmax - softmax.py
  • DotProductAttention, MultiHeadAttention - attention.py
  • TransformerLayer - transformer.py

@Wong4j
Wong4jforce-pushed the jaywan/add_nn_layer branch from 4840fba to 2ba9f21CompareAugust 6, 2023 16:39
@Wong4j

Wong4j commented Aug 7, 2023

Copy link
Copy Markdown
ContributorAuthor

Could you please help me review the code? @timmoon10
cc @jeng1220@Tom-Zheng

@Wong4jWong4j changed the title [WIP] [Paddle] Add nn layer[Paddle] Add nn layerAug 7, 2023
@zlsh80826

Copy link
Copy Markdown
Collaborator

/te-ci

2 similar comments
@Wong4j

Copy link
Copy Markdown
ContributorAuthor

/te-ci

@mingxu1067

Copy link
Copy Markdown
Collaborator

/te-ci

@Wong4j
Wong4jforce-pushed the jaywan/add_nn_layer branch from 2524f60 to 134d7a5CompareAugust 9, 2023 02:55
@mingxu1067

Copy link
Copy Markdown
Collaborator

/te-ci

Signed-off-by: Shijie Wang <jaywan@nvidia.com>
Signed-off-by: Shijie Wang <jaywan@nvidia.com>
@Wong4j
Wong4jforce-pushed the jaywan/add_nn_layer branch from 134d7a5 to 595a560CompareAugust 10, 2023 17:10
Signed-off-by: Shijie Wang <jaywan@nvidia.com>
@mingxu1067

Copy link
Copy Markdown
Collaborator

/te-ci

@jeng1220

Copy link
Copy Markdown
Contributor

LGTM.
@timmoon10 ,
All UT passed. Could you please review and merge this PR if everything looks good?

@jeng1220jeng1220 mentioned this pull request Aug 11, 2023

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall LGTM.

attention_dropout: float = 0.0,
attn_mask_type: str = "causal",
attention_type: str = "self",
backend: str = 'transformer_engine') -> None:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The PyTorch attention module has logic to automatically choose the backend based on what configurations are supported by Flash Attention or cuDNN. Instead of making the transformer_engine backend specific to cuDNN, how about we add a fused_attention backend and make the transformer_engine backend choose between paddle and fused_attention? If Flash Attention is ever ported to Paddle, we could also add a flash_attention backend. In general, I think it would be a good idea to move away from the strict dichotomy between transformer_engine and paddle backends.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure what you are saying.

This is TE/PyTorch:

classDotProductAttention(...):
if (condition):
# calling cuDNN fused attentionelse:
# calling flash_attention from hazyresearch

This is what we made for TE/PaddlePaddle for now:

classDotProductAttention(...):
if (condition):
# transformer_engine backed, calling cuDNN fused attentionelse:
# paddle backend, calling PaddlePaddle Primitives 

And what do you mean?

As far as I know, Baidu refers to flash_attention of hazyresearch and is developing its own version, but I am not sure the current status. Once complete, it will be in PaddlePaddle repo instead of hazyresearch repo.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The PyTorch behavior is more like:

classDotProductAttention(...):
if (condition1):
# calling flash_attention from hazyresearchelif (condition2):
# calling cuDNN fused attentionelse:
# calling PyTorch primitives

This decision is made during the forward pass, not at construction time, and it can change depending on the sequence lengths. Also, it's possible for the attention to call PyTorch primitives while other Transformer layer components use TE kernels.

@jeng1220jeng1220Aug 17, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So, there are 2 things.

  1. add 3rd option - calling flash_attention op (Baidu's version).
    I need to sync with Baidu to know their status. This will take some time. Also, Paddle primitives should be automatically converted to flash by the Paddle deep learning compiler if the compiler is ready, so we might not need the 3rd option. I think cuDNN is still our top priority.
  2. move the logic of kernel selecting from constructor into forward.
    It is easy to do because PaddlePaddle supports dynamic graph mode. Although it has maximum flexibility, it will block CUDA Graph integration if kernel can be dynamically changed at runtime, won't it?

I think both 1. and 2. need more time to have better understanding. Considering schedule pressure, I hope this PR can be merged first, then 1. and 2. can be follow up in the future.

If I misunderstand anything, please correct me.
Thanks

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yea, I think we can defer until later. Most of the backend logic in the PyTorch attention module is related to Flash Attention:

use_flash_attention=self.use_flash_attention
use_fused_attention=self.use_fused_attention
if (query_layer.dtypenotin [torch.bfloat16, torch.float16]
orkey_layer.dtypenotin [torch.bfloat16, torch.float16]
orvalue_layer.dtypenotin [torch.bfloat16, torch.float16]
):
use_flash_attention=False
ifkey_layer.shape[-1] >64:
ifself.device_compute_capabilityin (8.6, 8.7):
use_flash_attention=False
elifnot_flash_attn_2_availableandself.device_compute_capability==8.9:
use_flash_attention=False
ifnot_flash_attn_2_availableandself.num_gqa_groups!=self.num_attention_heads:
use_flash_attention=False
ifself.attn_mask_type=="padding"andattention_maskisnotNone:
use_flash_attention=False
use_fused_attention=False
ifcore_attention_bias_type!="no_bias"orcore_attention_biasisnotNone:
use_flash_attention=False
ifis_in_onnx_export_mode():
use_flash_attention=False
use_fused_attention=False

It seems that this logic changes rapidly, as the cuDNN team adds kernels or fixes bugs.

@ksivaman
ksivaman self-requested a review August 17, 2023 00:10
Comment threadtransformer_engine/paddle/layer/attention.py Outdated
Signed-off-by: Shijie Wang <jaywan@nvidia.com>
@Wong4j
Wong4jforce-pushed the jaywan/add_nn_layer branch from 5ff0d68 to e1f8ab2CompareAugust 17, 2023 11:33
Comment threadtransformer_engine/paddle/layer/attention.py Outdated
Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@ksivaman

Copy link
Copy Markdown
Member

/te-ci

@ksivaman
ksivaman merged commit 7444946 into NVIDIA:mainAug 17, 2023
ksivaman added a commit that referenced this pull request Aug 26, 2023
* Add nn.layer: softmax, attention, transformer
Signed-off-by: Shijie Wang <jaywan@nvidia.com>
* code refactor
Signed-off-by: Shijie Wang <jaywan@nvidia.com>
* code refactor
Signed-off-by: Shijie Wang <jaywan@nvidia.com>
* update docs and set dropout=0.1
Signed-off-by: Shijie Wang <jaywan@nvidia.com>
* Update transformer_engine/paddle/layer/attention.py
Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
---------
Signed-off-by: Shijie Wang <jaywan@nvidia.com>
Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@Wong4j@zlsh80826@mingxu1067@jeng1220@ksivaman@timmoon10