Skip to content

Comments

#13272: Vanilla UNet bring up#19405

Merged
dvartaniansTT merged 7 commits intomainfrom
keerthanar/vanilla_unet_bringup
Apr 9, 2025
Merged

#13272: Vanilla UNet bring up#19405
dvartaniansTT merged 7 commits intomainfrom
keerthanar/vanilla_unet_bringup

Conversation

@keerthana-r-mcw
Copy link
Contributor

@keerthana-r-mcw keerthana-r-mcw commented Mar 20, 2025 •
edited by sync-by-unito bot
Loading

Ticket

Link to Github Issue
#13272

Problem description

  • Add ttnn support for the Vanilla Unet model.

What's changed

  • Added Reference pipeline for Vanilla Unet.
  • Added ttnn support for the model.

Checklist

@keerthana-r-mcw keerthana-r-mcw mentioned this pull request Mar 25, 2025
Closed
@mbahnasTT mbahnasTT self-requested a review March 25, 2025 21:30
bbradelTT
bbradelTT approved these changes Mar 25, 2025
View reviewed changes
models/experimental/functional_vanilla_unet/README.md Outdated
Copy link
Contributor

@bbradelTT bbradelTT Mar 25, 2025

Choose a reason for hiding this comment

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

Consider changing to "refer to the ..."

TT-BrianLiu
TT-BrianLiu approved these changes Mar 25, 2025
View reviewed changes
models/experimental/functional_vanilla_unet/reference/unet.py Outdated
Copy link
Contributor

@TT-BrianLiu TT-BrianLiu Mar 25, 2025

Choose a reason for hiding this comment

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

Change to 2025 Tenstorrent ULC?

esmalTT
esmalTT requested changes Mar 25, 2025
View reviewed changes
Copy link
Contributor

@esmalTT esmalTT left a comment

Choose a reason for hiding this comment

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

Please add tests to the nightly pipeline and post CI results before requesting review

@keerthana-r-mcw keerthana-r-mcw force-pushed the keerthanar/vanilla_unet_bringup branch 5 times, most recently from a3a36f1 to 3a4c2d6 Compare March 27, 2025 06:47
@keerthana-r-mcw keerthana-r-mcw requested a review from a team as a code owner March 27, 2025 06:47
@keerthana-r-mcw keerthana-r-mcw force-pushed the keerthanar/vanilla_unet_bringup branch from 3a4c2d6 to 8a3353a Compare March 27, 2025 11:19
@keerthana-r-mcw keerthana-r-mcw requested a review from esmalTT March 27, 2025 13:10
@keerthana-r-mcw keerthana-r-mcw force-pushed the keerthanar/vanilla_unet_bringup branch from 8a3353a to ae96dd1 Compare April 1, 2025 12:49
@keerthana-r-mcw keerthana-r-mcw force-pushed the keerthanar/vanilla_unet_bringup branch from ae96dd1 to e10ee7e Compare April 3, 2025 06:49
esmalTT
esmalTT approved these changes Apr 6, 2025
View reviewed changes
Copy link
Contributor

@esmalTT esmalTT 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.

Looks good. 👍

Please address mine and @bbradelTT's comments before merging. You may want to run pipeline to make sure the changes are good before hitting merge.

In the future, I think we should avoid putting "functional" in the package name.

I don't think we'll ever have a slow and fast version of the model in the repository at once, and when the model graduates to "non-functional" it can cause a lot of unnecessary toil to remove the function prefix from the name.

models/experimental/functional_vanilla_unet/ttnn/common.py Outdated
return output_tensor


class Conv_split:
Copy link
Contributor

@esmalTT esmalTT Apr 6, 2025

Choose a reason for hiding this comment

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

Use TitleCase for class names in PEP8

Copy link
Contributor Author

@keerthana-r-mcw keerthana-r-mcw Apr 7, 2025

Choose a reason for hiding this comment

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

Sure

tt-rkim
tt-rkim reviewed Apr 6, 2025
View reviewed changes
tests/ttnn/integration_tests/vanilla_unet/test_ttnn_unet.py
@@ -0,0 +1,168 @@
# SPDX-FileCopyrightText: © 2025 Tenstorrent Inc.

# SPDX-License-Identifier: Apache-2.0
Copy link
Collaborator

@tt-rkim tt-rkim Apr 6, 2025

Choose a reason for hiding this comment

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

Does this test require the MLPerf model cache?

Copy link
Contributor

@ssinghalTT ssinghalTT Apr 8, 2025

Choose a reason for hiding this comment

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

@tt-rkim no. We are downloading weights on the fly.

@keerthana-r-mcw keerthana-r-mcw force-pushed the keerthanar/vanilla_unet_bringup branch from 415fa4d to 6a1313b Compare April 7, 2025 06:28
@keerthana-r-mcw keerthana-r-mcw force-pushed the keerthanar/vanilla_unet_bringup branch from 12d874c to 1a0139d Compare April 8, 2025 05:43
@ssinghalTT
Copy link
Contributor

ssinghalTT commented Apr 8, 2025

CIs:
https://github.com/tenstorrent/tt-metal/actions/runs/14334654183
https://github.com/tenstorrent/tt-metal/actions/runs/14322765646
https://github.com/tenstorrent/tt-metal/actions/runs/14322798313

@dvartaniansTT dvartaniansTT enabled auto-merge April 8, 2025 23:27
@dvartaniansTT dvartaniansTT added this pull request to the merge queue Apr 9, 2025
Merged via the queue into main with commit 678a464 Apr 9, 2025
22 checks passed
@dvartaniansTT dvartaniansTT deleted the keerthanar/vanilla_unet_bringup branch April 9, 2025 00:51
@saichandax saichandax mentioned this pull request Mar 8, 2025
Open
16 tasks
ssinghalTT added a commit that referenced this pull request Apr 23, 2025
@keerthana-r-mcw @ssinghalTT @dvartaniansTT
#13272: Vanilla UNet bring up (#19405)
06101a1 
### Ticket
Link to Github Issue
#13272
### Problem description
- Add ttnn support for the Vanilla Unet model.

### What's changed
- Added Reference pipeline for Vanilla Unet.
- Added ttnn support for the model.

### Checklist
- [x] [All post
commit](https://github.com/tenstorrent/tt-metal/actions/runs/14105906158)
CI passes
- [x] [(Single-card) Frequent model and ttnn
tests](https://github.com/tenstorrent/tt-metal/actions/runs/14100589034/job/39496118019)
CI passes (if applicable)

---------

Co-authored-by: Sudhanshu Singhal <ssinghal@tenstorrent.com>
Co-authored-by: Dalar Vartanians <132954887+dvartaniansTT@users.noreply.github.com>
Co-authored-by: Dalar Vartanians <dvartanians@tenstorrent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tt-rkim tt-rkim tt-rkim left review comments

@mbahnasTT mbahnasTT mbahnasTT approved these changes

@ssinghalTT ssinghalTT ssinghalTT left review comments

@TT-BrianLiu TT-BrianLiu TT-BrianLiu approved these changes

@uaydonat uaydonat uaydonat approved these changes

@esmalTT esmalTT esmalTT approved these changes

@bbradelTT bbradelTT bbradelTT approved these changes

@blozano-tt blozano-tt blozano-tt approved these changes

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

@dmakoviichuk-tt dmakoviichuk-tt Awaiting requested review from dmakoviichuk-tt

@rfurko-tt rfurko-tt Awaiting requested review from rfurko-tt

@cfjchu cfjchu Awaiting requested review from cfjchu

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

@dongjin-na dongjin-na Awaiting requested review from dongjin-na dongjin-na is a code owner

@omilyutin-tt omilyutin-tt Awaiting requested review from omilyutin-tt

@sminakov-tt sminakov-tt Awaiting requested review from sminakov-tt

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

@jmalone-tt jmalone-tt Awaiting requested review from jmalone-tt jmalone-tt 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.

10 participants

@keerthana-r-mcw @ssinghalTT @TT-BrianLiu @tt-rkim @mbahnasTT @uaydonat @esmalTT @bbradelTT @blozano-tt @dvartaniansTT