-
Notifications
You must be signed in to change notification settings - Fork 97
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
[UMD] Switching to new coord API #17003
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall lgtm, @tt-asaigal you might want to take a look as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall great changes! Just wondering - is the next step to have UMD accept generic coordinate systems so that we can get rid of convert_to_umd_coordinates
?
Yes, that would be one of the goals, it is questionable how many PRs I'll have until we come to that state, the existing code is messy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for starting this work out with a small PR, makes it easier to see the uplift
a1b2e91
to
2f7afa5
Compare
### Ticket Related to #17002 ### Problem description One of the many changes leading towards tt-metal using new CoreCoords, both in soc_descriptor and in umd's interface. Due to complexity of all the maps present, the changes will be relatively small. ### What's changed - virtual_core_from_physical_core and get_virtual_coordinate_from_physical_coordinates rewritten to use translate_coord_to instead of convert_to_umd_coordinates and translate_to_noc_table_coords - removed worker_log_to_physical_routing and physical_routing_to_virtual_routing from metal_soc_descriptor - get_physical_tensix_core_from_logical and convert_to_umd_coordinates rewritten to use translate_coord_to - Rewritten some get_physical_core_from_logical_core to get_physical_tensix_core_from_logical - grayskull soc descriptor changed so that it is rowwise instead of columnwise. This matches the way it is written in UMD, and it matches other soc descriptors. - bump umd to pull in some new changes in UMD ### Checklist All runs on brosko/soc_new_api : - [x] All post-commit tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986193862 - [x] Blackhole post-commit tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986195849 - [x] (Single-card) Model perf tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986198348 - [x] (Single-card) Device perf regressions : https://github.com/tenstorrent/tt-metal/actions/runs/12986201144 - [x] (T3K) T3000 unit tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986203715 - [x] (T3K) T3000 demo tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986205723 - [x] (TG) TG unit tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986207280 - [x] (TG) TG demo tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986209173 - [x] (TGG) TGG unit tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986211545 - [x] (TGG) TGG demo tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986213812
### Ticket Related to #17002 ### Problem description One of the many changes leading towards tt-metal using new CoreCoords, both in soc_descriptor and in umd's interface. Due to complexity of all the maps present, the changes will be relatively small. ### What's changed - virtual_core_from_physical_core and get_virtual_coordinate_from_physical_coordinates rewritten to use translate_coord_to instead of convert_to_umd_coordinates and translate_to_noc_table_coords - removed worker_log_to_physical_routing and physical_routing_to_virtual_routing from metal_soc_descriptor - get_physical_tensix_core_from_logical and convert_to_umd_coordinates rewritten to use translate_coord_to - Rewritten some get_physical_core_from_logical_core to get_physical_tensix_core_from_logical - grayskull soc descriptor changed so that it is rowwise instead of columnwise. This matches the way it is written in UMD, and it matches other soc descriptors. - bump umd to pull in some new changes in UMD ### Checklist All runs on brosko/soc_new_api : - [x] All post-commit tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986193862 - [x] Blackhole post-commit tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986195849 - [x] (Single-card) Model perf tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986198348 - [x] (Single-card) Device perf regressions : https://github.com/tenstorrent/tt-metal/actions/runs/12986201144 - [x] (T3K) T3000 unit tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986203715 - [x] (T3K) T3000 demo tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986205723 - [x] (TG) TG unit tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986207280 - [x] (TG) TG demo tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986209173 - [x] (TGG) TGG unit tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986211545 - [x] (TGG) TGG demo tests : https://github.com/tenstorrent/tt-metal/actions/runs/12986213812
Ticket
Related to #17002
Problem description
One of the many changes leading towards tt-metal using new CoreCoords, both in soc_descriptor and in umd's interface.
Due to complexity of all the maps present, the changes will be relatively small.
What's changed
Checklist
All runs on brosko/soc_new_api :