Different dimension for attention by pggPL · Pull Request #833 · NVIDIA/TransformerEngine

pggPL · 2024-05-03T20:12:07Z

Description

I added channels_per_head parameter to attention. I know that it is breaking change, but I think there is not a good way of doing it - look at the discussion in #731

Fixes # (issue)

Type of change

Documentation change (change only to the documentation, either a fix or a new content)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Changes

Please list the changes introduced in this PR:

Added channels_per_head parameter to attention,
Added simple unit test, I don't know if its necessary

Checklist:

I have read and followed the contributing guidelines
The functionality is complete
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com> Signed-off-by: root <root@ipp2-0037.nvidia.com>

Signed-off-by: root <root@ipp2-0037.nvidia.com>

…ghts - now loading works correctly, confirmed by the generation code. Signed-off-by: root <root@ipp2-1661.nvidia.com>

Signed-off-by: root <root@ipp2-1661.nvidia.com>

Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

ptrendx · 2024-05-04T00:06:41Z

I think this is unintended change?

Yes, I will remove it.

No, apparently it's ok. In last commit which was merged it was accidentally changed, so we want to revert it.

ptrendx · 2024-05-04T00:08:36Z

 cp_group: Optional[dist_group_type] = None,
 cp_global_ranks: List[int] = None,
 cp_stream: torch.cuda.Stream = None,
+ kv_channels: int = None # deprecated


Well, if it is deprecated, then we should make sure that the code that explicitly calls kv_channels=something still works as before, right? While those changes are just ignoring it.

I think the code will crash when kv_channels were set for different value than default, so we can safely do this. We have in the code
self.hidden_size_kv = int(hidden_size * self.num_gqa_groups // num_attention_heads) ... self.layernorm_qkv = LayerNormLinear( hidden_size, hidden_size + 2 * self.hidden_size_kv, ...
So the input size to the attention does not depend from kv_channels - do kv_channels can have one value.
I see this is not the case with DotProductAttention, I will look at this.
Nevertheless, I want to ask if I want to deprecate an argument, what should I do? Will causing it to behave the same at previously, but removing from docs will be ok? Do we consider it as breaking the API?

Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

sudhakarsingh27

The tutorial file's mode has been changed. Could you pls remove that change?

pggPL · 2024-05-14T01:11:52Z

But this tutorial mode change is good. Should I remove it and add to the new PR?

Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

…TransformerEngine into different_dimension_for_attention

Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

sudhakarsingh27 · 2024-05-16T03:23:23Z

/te-ci pytorch

ksivaman · 2024-05-17T18:14:14Z

/te-ci pytorch

Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

…TransformerEngine into different_dimension_for_attention

Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

…or_attention

sudhakarsingh27 · 2024-05-21T19:46:40Z

/te-ci pytorch

ksivaman

Why change the file permission for the tutorial?

Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

sudhakarsingh27 · 2024-05-24T17:18:14Z

Why change the file permission for the tutorial?

It seems like we mistakenly changed the permission last time so I think we're reverting it back. Is that okay or do we need to create a separate fix for it?

sudhakarsingh27 · 2024-05-24T17:18:57Z

/te-ci pytorch

rootand others added 22 commits March 27, 2024 00:55

Fixed Llama tutorial. Changed batch size and added fused=True.
7f386e7
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com> Signed-off-by: root <root@ipp2-0037.nvidia.com>

Tutorial updated but not complete yet.
55ee043
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com> Signed-off-by: root <root@ipp2-0037.nvidia.com>

Tutorial notebook reseted - removed fuse=true
c1f7855
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com> Signed-off-by: root <root@ipp2-0037.nvidia.com>

Removed fused=true
2ddf164
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com> Signed-off-by: root <root@ipp2-0037.nvidia.com>

Batch size back to 8
b2405fb
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com> Signed-off-by: root <root@ipp2-0037.nvidia.com>

Typo and commented out line
557d040
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com> Signed-off-by: root <root@ipp2-0037.nvidia.com>

fixed whitespace
eccef63
Signed-off-by: root <root@ipp2-0037.nvidia.com>

fixed whitespace
12fe37c
Signed-off-by: root <root@ipp2-0037.nvidia.com>

Added comment to attention line. Fixed potential bug with loading wei…
80dda95
…ghts - now loading works correctly, confirmed by the generation code. Signed-off-by: root <root@ipp2-1661.nvidia.com>

Comments
90e496e
Signed-off-by: root <root@ipp2-1661.nvidia.com>

Models cast added again
9985578
Signed-off-by: root <root@ipp2-1661.nvidia.com>

Weight download info
0cfbd21
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Moved parameter gate_proj_size to config
ee10c1c
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

gate_proj_size removed and put immediate_size instead
4a734a7
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Merge branch 'main' of github.com:NVIDIA/TransformerEngine
1708696

Merge branch 'main' of github.com:NVIDIA/TransformerEngine
620a685

Llama 3 added to tutorial
0010ade
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Typos fix
36b7110
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Typos fix
5846e77
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Fixed model loading
6f9a358
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Loading fix
7085763
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Merge branch 'main' of github.com:NVIDIA/TransformerEngine
77a3bc0

pggPL marked this pull request as draft May 3, 2024 20:12

pggPL mentioned this pull request May 3, 2024
Fix of bug of kv_channels in TransformerLayer and added Gemma tutorial #731
Closed

pggPL added 2 commits May 3, 2024 20:33

Different dim for attention
edb9e9f
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Reversed other commit
e1500ae
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

ptrendx reviewed May 4, 2024
View reviewed changes

pggPL added 2 commits May 9, 2024 12:34

Changed name to kv_channels
65ec00e
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Fixed typo
a38e124
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

pggPLand others added 5 commits May 9, 2024 12:43

Back to kv_channels in transformer layer
d72934c
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Back to kv_channels in transformer layer
4332e51
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Small bug fix
e6b529a
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Small bug fix
9da3435
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Merge branch 'main' into different_dimension_for_attention
74074fb

sudhakarsingh27 reviewed May 13, 2024
View reviewed changes

Comment threadtests/pytorch/test_gqa.py

pggPL added 2 commits May 13, 2024 18:37

Test fix
df835eb
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Merge branch 'different_dimension_for_attention' of github.com:pggPL/…
c36edec
…TransformerEngine into different_dimension_for_attention

pggPL marked this pull request as ready for review May 14, 2024 01:39

pggPLand others added 2 commits May 15, 2024 16:45

changed file modes
364ae35
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Merge branch 'main' into different_dimension_for_attention
1d4bf8e

sudhakarsingh27 approved these changes May 16, 2024
View reviewed changes

Merge branch 'main' into different_dimension_for_attention
e3b72b7

pggPL added 4 commits May 20, 2024 09:42

lint fix and resolved conflict
d775d18
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Merge branch 'different_dimension_for_attention' of github.com:pggPL/…
79b3256
…TransformerEngine into different_dimension_for_attention

lint fix and resolved conflict
c23d24f
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

Merge remote-tracking branch 'origin/main' into different_dimension_f…
8312e14
…or_attention

ksivaman reviewed May 21, 2024
View reviewed changes

Lint fix, hopefully last
ce94bfc
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>

sudhakarsingh27 merged commit 6673689 into NVIDIA:mainMay 25, 2024

pggPL deleted the different_dimension_for_attention branch August 5, 2026 16:16

Uh oh!

Conversation

pggPL commented May 3, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Changes

Checklist:

Uh oh!

ptrendxMay 4, 2024

Choose a reason for hiding this comment

Uh oh!

pggPLMay 6, 2024

Choose a reason for hiding this comment

Uh oh!

pggPLMay 9, 2024

Choose a reason for hiding this comment

Uh oh!

ptrendxMay 4, 2024

Choose a reason for hiding this comment

Uh oh!

pggPLMay 6, 2024

Choose a reason for hiding this comment

Uh oh!

sudhakarsingh27 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pggPL commented May 14, 2024

Uh oh!

sudhakarsingh27 commented May 16, 2024

Uh oh!

ksivaman commented May 17, 2024

Uh oh!

sudhakarsingh27 commented May 21, 2024

Uh oh!

ksivaman left a comment

Choose a reason for hiding this comment

Uh oh!

sudhakarsingh27 commented May 24, 2024

Uh oh!

sudhakarsingh27 commented May 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pggPL commented May 3, 2024•
edited
Loading