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

detect_arch in ClusterDescriptor #345

Merged
merged 6 commits into from
Dec 4, 2024
Merged

detect_arch in ClusterDescriptor #345

merged 6 commits into from
Dec 4, 2024

Conversation

broskoTT
Copy link
Contributor

@broskoTT broskoTT commented Nov 27, 2024

Issue

Related to #99
Aftermath of #175

Description

Make detect_arch available through cluster_descriptor which should be the only real endpoint when reading information about chips in the cluster.

List of the changes

  • Removed detect_arch from cluster.h
  • Added arch map in ClusterDescriptor
  • Added static tt_ClusterDescriptor::detect_arch which should be used instead of previously defined detect_arch

Testing

Wrote a test which verifies this works as intended

API Changes

This PR has API changes:

@broskoTT broskoTT requested a review from pjanevskiTT November 27, 2024 11:16
device/tt_cluster_descriptor.cpp Show resolved Hide resolved
tests/api/test_cluster_descriptor.cpp Show resolved Hide resolved
device/tt_cluster_descriptor.cpp Outdated Show resolved Hide resolved
device/tt_cluster_descriptor.cpp Outdated Show resolved Hide resolved
@broskoTT broskoTT force-pushed the brosko/cluster_desc_no_yaml branch from a61e27a to 87308c9 Compare November 28, 2024 08:43
@broskoTT broskoTT added the changes api API changing PR, needs changes in client code label Nov 28, 2024
Base automatically changed from brosko/cluster_desc_no_yaml to main November 29, 2024 07:54
Copy link
Contributor

@pjanevskiTT pjanevskiTT left a comment

Choose a reason for hiding this comment

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

thanks for the changes

@broskoTT broskoTT merged commit dca4e49 into main Dec 4, 2024
19 of 22 checks passed
@broskoTT broskoTT deleted the brosko/detect_arch branch December 4, 2024 16:34
broskoTT added a commit that referenced this pull request Dec 5, 2024
broskoTT added a commit that referenced this pull request Dec 5, 2024
broskoTT added a commit to tenstorrent/tt-metal that referenced this pull request Dec 5, 2024
### Ticket
Related to #13948

### Problem description
detect_arch was in the global namespace. 
Now putting this functionality in the right place. It also now works
with logical ids, as opposed to pci device enumeration id.
Related UMD change tenstorrent/tt-umd#345

### What's changed
Changed detect_arch to tt_ClusterDescriptor::get_arch()
BoardType::DEFAULT changed to UNKNOWN
Also changed detect_arch to PCIDevice::enumerate_devices_info(), due to
ClusterDescriptor::create() not working.

### Checklist
- [x] All post-commit tests :
https://github.com/tenstorrent/tt-metal/actions/runs/12139731818
- [x] Blackhole post-commit tests :
https://github.com/tenstorrent/tt-metal/actions/runs/12139733483
- [ ] (Single-card) Model perf tests :
https://github.com/tenstorrent/tt-metal/actions/runs/12139735518
- [x] (Single-card) Device perf regressions :
https://github.com/tenstorrent/tt-metal/actions/runs/12139737448
- [x] (T3K) T3000 unit tests :
https://github.com/tenstorrent/tt-metal/actions/runs/12139739342
- [x] (T3K) T3000 demo tests :
https://github.com/tenstorrent/tt-metal/actions/runs/12139741041
- [x] (TG) TG unit tests :
https://github.com/tenstorrent/tt-metal/actions/runs/12139742806
- [x] (TG) TG demo tests :
https://github.com/tenstorrent/tt-metal/actions/runs/12139744773
- [ ] (TGG) TGG unit tests :
https://github.com/tenstorrent/tt-metal/actions/runs/12139746353
- [x] (TGG) TGG demo tests :
https://github.com/tenstorrent/tt-metal/actions/runs/12139748257
- [x] All post-commit tests on last commit:
https://github.com/tenstorrent/tt-metal/actions/runs/12178722655
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.

2 participants