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

Add perf reporting for ccl async mode #16658

Merged
merged 5 commits into from
Jan 30, 2025
Merged

Add perf reporting for ccl async mode #16658

merged 5 commits into from
Jan 30, 2025

Conversation

Aswinmcw
Copy link
Contributor

@Aswinmcw Aswinmcw commented Jan 13, 2025 •
edited
Loading

Ticket

#16648

Problem description

Need perf reporting for async all gather

What's changed

Screenshot 2025-01-24 at 8 42 29 PM

Checklist

  • Post commit CI passes
  • Blackhole Post commit (if applicable)
  • Model regression CI testing passes (if applicable)
  • Device performance regression CI testing passes (if applicable)
  • (For models and ops writers) Full new models tests passes
  • New/Existing tests provide coverage for changes

@Aswinmcw Aswinmcw force-pushed the Aswinmcw/ccl_async_perf branch 4 times, most recently from 223273e to a284b8f Compare January 21, 2025 06:31
@Aswinmcw Aswinmcw force-pushed the Aswinmcw/ccl_async_perf branch from a284b8f to 52bf222 Compare January 24, 2025 08:48
@Aswinmcw Aswinmcw requested a review from SeanNijjar January 24, 2025 13:40
SeanNijjar
SeanNijjar reviewed Jan 24, 2025
View reviewed changes
Copy link
Contributor

@SeanNijjar SeanNijjar left a comment

Choose a reason for hiding this comment

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

Thanks Aswin! Minor request for additional specific test cases

tests/ttnn/unit_tests/operations/ccl/perf/test_ccl_async_perf.py Outdated
"num_devices, num_links, output_shape, dim, layout",
[
(4, 1, [1, 1, 64, 512], 3, ttnn.TILE_LAYOUT),
# (4, 1, [1, 1, 32, 32768], 3, ttnn.TILE_LAYOUT),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add the following shapes/configurations (each with bfp8 and fp16)?

4-chip, in_shape :[1,1,32,1280], dim=0
4-chip, in_shape:[1,1,32,7168], dim=0
8-chip, in_shape:[1,1,32,2048], dim=0
4-chip, in_shape:[1,1,32,3584], dim=0
4-chip, in_shape:[1,1,32,32],dim=0
4-chip, in_shape:[1,1,8,32],dim=2 // This one may not work yet with the new all-gather. If it doesn't we can add a separate issue to add the support for padded tiles

Copy link
Contributor

@SeanNijjar SeanNijjar Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, can you please also add equivalent tests for TG? There are some example tests of the cluster axis API usage in this test file.

@Aswinmcw Aswinmcw force-pushed the Aswinmcw/ccl_async_perf branch 2 times, most recently from 2ccff5a to 062b5c1 Compare January 27, 2025 11:09
@Aswinmcw Aswinmcw requested a review from SeanNijjar January 27, 2025 14:51
@Aswinmcw Aswinmcw force-pushed the Aswinmcw/ccl_async_perf branch from 062b5c1 to 20de603 Compare January 28, 2025 12:00
SeanNijjar
SeanNijjar reviewed Jan 28, 2025
View reviewed changes
tests/ttnn/unit_tests/operations/ccl/test_new_all_gather.py Outdated
@@ -168,7 +168,7 @@ def run_all_gather_impl(

# create global semaphore handles
ccl_semaphore_handles = create_global_semaphore_with_same_address(mesh_device, ccl_sub_device_crs, 0)

output_shape[dim] *= num_devices
Copy link
Contributor

Choose a reason for hiding this comment

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

this should get moved to the caller where your new tests are - otherwise you'll need to update every other test to start passing input shape instead.

@Aswinmcw Aswinmcw force-pushed the Aswinmcw/ccl_async_perf branch from 20de603 to 7e1705f Compare January 29, 2025 07:22
SeanNijjar
SeanNijjar approved these changes Jan 29, 2025
View reviewed changes
tests/ttnn/unit_tests/operations/ccl/perf/test_ccl_async_perf.py Outdated
(8, 1, [1, 1, 32, 2048], 0, ttnn.TILE_LAYOUT),
(4, 1, [1, 1, 32, 3584], 0, ttnn.TILE_LAYOUT),
(4, 1, [1, 1, 32, 32], 0, ttnn.TILE_LAYOUT),
# (4, 1, [1, 1, 8, 32], 2, ttnn.TILE_LAYOUT),
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove commented out cases

@Aswinmcw Aswinmcw force-pushed the Aswinmcw/ccl_async_perf branch from 7e1705f to d51707d Compare January 30, 2025 05:07
@Aswinmcw Aswinmcw marked this pull request as ready for review January 30, 2025 07:43
@Aswinmcw Aswinmcw merged commit 32bb829 into main Jan 30, 2025
197 of 210 checks passed
@Aswinmcw Aswinmcw deleted the Aswinmcw/ccl_async_perf branch January 30, 2025 08:03
williamlyTT pushed a commit that referenced this pull request Jan 30, 2025
@Aswinmcw @williamlyTT
Add perf reporting for ccl async mode (#16658)
826e408 
### Ticket
#16648 

### Problem description
Need perf reporting for async all gather

### What's changed
<img width="1292" alt="Screenshot 2025-01-24 at 8 42 29 PM"
src="https://github.com/user-attachments/assets/33c64cf1-9f31-4567-89ed-813acc16e49d"
/>


### Checklist
- [ ] Post commit CI passes
- [ ] Blackhole Post commit (if applicable)
- [ ] Model regression CI testing passes (if applicable)
- [ ] Device performance regression CI testing passes (if applicable)
- [ ] **(For models and ops writers)** Full [new
models](https://github.com/tenstorrent/tt-metal/actions/workflows/full-new-models-suite.yaml)
tests passes
- [ ] New/Existing tests provide coverage for changes
yieldthought pushed a commit that referenced this pull request Jan 31, 2025
@Aswinmcw @yieldthought
Add perf reporting for ccl async mode (#16658)
08b7e9e 
### Ticket
#16648 

### Problem description
Need perf reporting for async all gather

### What's changed
<img width="1292" alt="Screenshot 2025-01-24 at 8 42 29 PM"
src="https://github.com/user-attachments/assets/33c64cf1-9f31-4567-89ed-813acc16e49d"
/>


### Checklist
- [ ] Post commit CI passes
- [ ] Blackhole Post commit (if applicable)
- [ ] Model regression CI testing passes (if applicable)
- [ ] Device performance regression CI testing passes (if applicable)
- [ ] **(For models and ops writers)** Full [new
models](https://github.com/tenstorrent/tt-metal/actions/workflows/full-new-models-suite.yaml)
tests passes
- [ ] New/Existing tests provide coverage for changes
nikileshx pushed a commit to nikileshx/tt-metal that referenced this pull request Feb 3, 2025
@Aswinmcw @nikileshx
Add perf reporting for ccl async mode (tenstorrent#16658)
fe09b77 
### Ticket
tenstorrent#16648 

### Problem description
Need perf reporting for async all gather

### What's changed
<img width="1292" alt="Screenshot 2025-01-24 at 8 42 29 PM"
src="https://github.com/user-attachments/assets/33c64cf1-9f31-4567-89ed-813acc16e49d"
/>


### Checklist
- [ ] Post commit CI passes
- [ ] Blackhole Post commit (if applicable)
- [ ] Model regression CI testing passes (if applicable)
- [ ] Device performance regression CI testing passes (if applicable)
- [ ] **(For models and ops writers)** Full [new
models](https://github.com/tenstorrent/tt-metal/actions/workflows/full-new-models-suite.yaml)
tests passes
- [ ] New/Existing tests provide coverage for changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SeanNijjar SeanNijjar SeanNijjar approved these changes

@jvegaTT jvegaTT Awaiting requested review from jvegaTT jvegaTT is a code owner

@tt-aho tt-aho Awaiting requested review from tt-aho tt-aho is a code owner

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

Successfully merging this pull request may close these issues.
2 participants
@Aswinmcw @SeanNijjar