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.

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

Rename tt_SiliconDevice to tt::umd::Cluster #265

Merged
merged 5 commits into from
Nov 14, 2024
Merged

Conversation

broskoTT
Copy link
Contributor

@broskoTT broskoTT commented Nov 5, 2024

Fixes #108
For bigger picture, see https://docs.google.com/drawings/d/1-m1azdsBqMA0A6ATYRMfkhyeuOJuGCEI62N5a96LXj0/edit

Changes:

  • Renamed tt_SiliconDevice to Cluster
  • Added Cluster to tt::umd namespace.
  • Renamed tt_device.h to cluster.h, and tt_silicon_driver.cpp to cluster.cpp
  • Moved a few trivial functions from tt_device.cpp to cluster.cpp.

Note that tt_device will be removed, according to the diagram. Currently, simulation device and mockup device are specializations of current tt::umd::Cluster. When we create tt::umd::Chip abstraction, they will be moved correspondingly, and this tt_device abstraction will be removed.

API Changes

This PR has API changes:

@joelsmithTT
Copy link
Contributor

Currently, simulation device and mockup device are specializations of current tt::umd::Cluster. When we create tt::umd::Chip abstraction, they will be moved correspondingly, and this tt_device abstraction will be removed.

Can you elaborate a bit on this? Simulation & Mockup both inherit tt_cluster, as does tt::umd::Cluster (formerly tt_SiliconDevice). Are you saying that a single tt::umd::Cluster will handle all cases (mockup, simulation, silicon) depending on the tt::umd::Chip subclasses that are instantiated at runtime?

Do you think there is value in tt_device -> ICluster or AbstractCluster (i.e. maintain an abstract Cluster interface for testing purposes)?

Base automatically changed from brosko/move_from_ttdevice to main November 5, 2024 17:21
@broskoTT
Copy link
Contributor Author

broskoTT commented Nov 5, 2024

@joelsmithTT
For the first part, yes you got it right.

For the second part we can change it if we chose to implement those tests. It does make sense, we should consider it.

In any case, according to the diagram, the Cluster class will be publicly facing class, which will have ClusterInternal holding implementation (the pimpl design pattern). The ClusterInternal would be a good candidate for this.

@broskoTT broskoTT added the changes api API changing PR, needs changes in client code label Nov 5, 2024
@broskoTT broskoTT force-pushed the brosko/rename_cluster branch 2 times, most recently from dcf36e9 to ded6eb5 Compare November 13, 2024 12:03
@broskoTT broskoTT merged commit 8985708 into main Nov 14, 2024
19 checks passed
@broskoTT broskoTT deleted the brosko/rename_cluster branch November 14, 2024 14:05
broskoTT added a commit to tenstorrent/tt-metal that referenced this pull request Nov 15, 2024
### Ticket
Related to #13948

### What's changed
Related UMD PR: tenstorrent/tt-umd#265
Changed naming
tt_device.h -> cluster.h
tt_SiliconDriver -> tt::umd::Cluster

### Checklist
- [x] Post commit CI passes:
https://github.com/tenstorrent/tt-metal/actions/runs/11846636922
- [x] Blackhole Post commit (if applicable):
https://github.com/tenstorrent/tt-metal/actions/runs/11839014934
- [x] Model regression CI testing passes (if applicable)
- [x] Device performance regression CI testing passes (if applicable)
- [x] 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
Labels
changes api API changing PR, needs changes in client code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename tt_SiliconDriver to tt::umd::Cluster
2 participants