Skip to content

Lowering aten.native_group_norm.default #556

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

swimdi
Copy link
Contributor

@swimdi swimdi commented Dec 4, 2024

Ticket

#538

Problem description

Support the lowering of aten.native_group_norm.default

I implement it accroding to test_group_norm.py on tt-metal

Originally I reference the test_group_norm_with_block_sharded_v2_8x8_grid_tile_layout part, but some cases accuracy failed

"input_shape, num_groups",
[(1, 320, 32, 32), 32]
[(2, 320, 64, 64), 32]

And after I set the input_tensor layout as TtnnRowMajorLayout, its accuracy pass

And there still some case failed, I issued at #555

What's changed

  • Support lowering aten.native_group_norm.default
  • Add aten_native_group_norm_default_blocklist
  • Add test_group_norm.py

btw, only stable diffusion v2 & retinanet_resnet50_fpn_v2 use group_norm, so maybe this op's priority can lower

I'll merge #532 first and then this PR

@swimdi swimdi linked an issue Dec 4, 2024 that may be closed by this pull request
@ayerofieiev-tt
Copy link
Member

And after I set the input_tensor layout as TtnnRowMajorLayout, its accuracy pass

Please fire a ticket. There should be no requirement to change from Tile to RM for PCC to pass. Sounds like a bug to me.

@swimdi
Copy link
Contributor Author

swimdi commented Dec 5, 2024

And after I set the input_tensor layout as TtnnRowMajorLayout, its accuracy pass

Please fire a ticket. There should be no requirement to change from Tile to RM for PCC to pass. Sounds like a bug to me.

that case is mentioned in #555 , or you prefer an individual issue@@?

Copy link
Contributor

@jerrysky3 jerrysky3 left a comment

Choose a reason for hiding this comment

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

LGTM on the group norm parts added by this PR

grid_size_x = device.compute_with_storage_grid_size().x
grid_size_y = device.compute_with_storage_grid_size().y
shard_shape = N * H * W // grid_size_x, C // grid_size_y
if shard_shape[0] == 0 or shard_shape[1] == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a TODO to remind us the shard shape computation can be improved later

@swimdi swimdi self-assigned this Dec 6, 2024
@swimdi swimdi force-pushed the swimdi/stage4-group_norm branch from 5a5790e to 78b8945 Compare December 6, 2024 04:28
@swimdi swimdi added this pull request to the merge queue Dec 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 6, 2024
@swimdi swimdi force-pushed the swimdi/stage4-group_norm branch from 78b8945 to f59193f Compare December 11, 2024 07:15
@swimdi swimdi enabled auto-merge December 11, 2024 07:16
@swimdi swimdi added this pull request to the merge queue Dec 11, 2024
@swimdi swimdi removed this pull request from the merge queue due to a manual request Dec 11, 2024
@swimdi swimdi added this pull request to the merge queue Dec 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 11, 2024
@swimdi swimdi force-pushed the swimdi/stage4-group_norm branch from f59193f to 5822449 Compare December 17, 2024 02:34
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.

aten.native_group_norm.default
3 participants