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

Implement Cluster CoreCoord API #401

Merged
merged 5 commits into from
Dec 20, 2024
Merged

Implement Cluster CoreCoord API #401

merged 5 commits into from
Dec 20, 2024

Conversation

pjanevskiTT
Copy link
Contributor

Issue

#220)

Description

Provide Cluster API with CoreCoord. Every function that was expecting virtual coordinates has the same function just with CoreCoord structure. Old API is still there just to not break tt-metal and tt-lens at the moment. When the clients have switched to the new API we can delete the old one. In the meantime, internals of Cluster can be redesigned in parallel with client work on switching to new API. So this should be a two phase change. This is the first one where the new API is added, and when clients have switched to the new API second phase will be deleting the old API

List of the changes

  • Separate Cluster API into 3 groups (something we want to keep, to delete and new API)
  • Add API with CoreCoord
  • Change API tests to use the new API

Testing

UMD unit tests

API Changes

Old API is still there, so no API changes at the moment

@pjanevskiTT
Copy link
Contributor Author

@pgkeller

Base automatically changed from pjanevski/tt_soc_descriptor_api to main December 16, 2024 12:51
@pjanevskiTT pjanevskiTT force-pushed the pjanevski/core_coord_api branch from 1d8d2b5 to f9833cd Compare December 16, 2024 13:11
Copy link

@pgkeller pgkeller left a comment

Choose a reason for hiding this comment

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

looks good overall to me, just a couple questions about "to".
please be sure to get Almeet's review

device/api/umd/device/cluster.h Outdated Show resolved Hide resolved
Copy link
Contributor

@broskoTT broskoTT left a comment

Choose a reason for hiding this comment

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

Looking forward to metal team using the new api...

device/api/umd/device/cluster.h Outdated Show resolved Hide resolved
device/api/umd/device/cluster.h Outdated Show resolved Hide resolved
device/cluster.cpp Outdated Show resolved Hide resolved
device/api/umd/device/cluster.h Outdated Show resolved Hide resolved
device/api/umd/device/tt_core_coordinates.h Outdated Show resolved Hide resolved
device/api/umd/device/tt_core_coordinates.h Show resolved Hide resolved
tests/api/test_cluster.cpp Outdated Show resolved Hide resolved
tests/api/test_cluster.cpp Outdated Show resolved Hide resolved
tests/api/test_cluster.cpp Outdated Show resolved Hide resolved
tests/api/test_cluster.cpp Outdated Show resolved Hide resolved
device/api/umd/device/cluster.h Outdated Show resolved Hide resolved
device/cluster.cpp Show resolved Hide resolved
device/cluster.cpp Show resolved Hide resolved
device/api/umd/device/cluster.h Show resolved Hide resolved
device/cluster.cpp Show resolved Hide resolved
@pjanevskiTT pjanevskiTT force-pushed the pjanevski/core_coord_api branch 3 times, most recently from fc5735d to fc407bc Compare December 17, 2024 15:38
@pjanevskiTT pjanevskiTT force-pushed the pjanevski/core_coord_api branch 7 times, most recently from 998893a to cf27226 Compare December 18, 2024 17:09
device/api/umd/device/tt_soc_descriptor.h Show resolved Hide resolved
device/cluster.cpp Show resolved Hide resolved
tests/api/test_cluster.cpp Show resolved Hide resolved
@pjanevskiTT pjanevskiTT force-pushed the pjanevski/core_coord_api branch from c3f0293 to 1bd57f2 Compare December 20, 2024 12:05
@pjanevskiTT pjanevskiTT enabled auto-merge (squash) December 20, 2024 12:12
@pjanevskiTT pjanevskiTT merged commit e092e10 into main Dec 20, 2024
19 checks passed
@pjanevskiTT pjanevskiTT deleted the pjanevski/core_coord_api branch December 20, 2024 12:31
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.

5 participants