Soc descriptor harvesting

pjanevskiTT commented 2 weeks ago

Fix https://github.com/tenstorrent/tt-umd/issues/102, part of a general effort around soc descriptor https://github.com/tenstorrent/tt-umd/issues/100

Goal of the PR is to move harvesting logic to soc descriptor, with clear interface for coordinate translation

Finished TODOs

[x] Derive logical, virtual and physical coordinates from tt_xy_pair
[x] Make soc descriptor tests arch independent as much as possible
[x] Cleanup soc desc code for harvesting on y or x based on the arch
[x] Revert commented tests
[x] Move open UMD doc about harvesting to UMD repo
[x] Add tests for grayskull
[x] Full interface to convert between cordinate system

First class TODOs (will finish in this PR)

[x] Blackhole to translated coordinates fix
[x] cpp nits

UMD client changes

No changes needed for bumping

TODOs with follow up issues (won't fix in this PR)

[ ] Cleanup coordinate system docs - issue #217
[ ] Cover non-worker cores (DRAM, eth, arc) - issue https://github.com/tenstorrent/tt-umd/issues/219
[ ] Compile time polymorphism for coordinate types - issue https://github.com/tenstorrent/tt-umd/issues/221
[ ] Document CoordinateManager classes - issue #216
[ ] Confirm that std::map for mapping physical coordinates doesn't have big perf penalty -issue https://github.com/tenstorrent/tt-umd/issues/222
[ ] Provide tt_SiliconDevice API with all types of coordinates - issue https://github.com/tenstorrent/tt-umd/issues/220

abhullar-tt commented 2 weeks ago

is the idea for tt_xy_pair to have an additional field to denote coordinate type (logical, virtual, physical, etc?)

pjanevskiTT commented 2 weeks ago

is the idea for tt_xy_pair to have an additional field to denote coordinate type (logical, virtual, physical, etc?)

I am probably going to derive each type of coordinates from tt_xy_pair, so instead of

using tt_logical_coords = tt::umd::tt_xy_pair;

we are going to have derive tt_logical_coords from tt_xy_pair, so you still have only x and y as fields, but you can use coordinates as types, you will know which coordinates you are using just based on the type of the variable. Also, we want to be sure to not use the API in a way it is not intended, we can provide API for each type of coordinates. Does that make more sense?

abhullar-tt commented 2 weeks ago

is the idea for tt_xy_pair to have an additional field to denote coordinate type (logical, virtual, physical, etc?)

I am probably going to derive each type of coordinates from tt_xy_pair, so instead of
using tt_logical_coords = tt::umd::tt_xy_pair;
we are going to have derive tt_logical_coords from tt_xy_pair, so you still have only x and y as fields, but you can use coordinates as types, you will know which coordinates you are using just based on the type of the variable. Also, we want to be sure to not use the API in a way it is not intended, we can provide API for each type of coordinates. Does that make more sense?

yes that makes sense, thanks!

pjanevskiTT commented 1 week ago

I know this PR is becoming a bit hard to review with a lot of code, so I will separate TODOs left into first class that I will solve in this PR and then merge it, and into second class tasks that I will open issues for and solve it all as separate PRs. I tried thinking about making this PR smaller as well, but I think this is a good base (coordinate conversion API, tests, docs) for harvesting so we can build on top of it. Uplifting this is also going to be seamless since it doesn't change or removes the API, it just adds the functions, so I can make the API function for tt-metal in the following commits.

@broskoTT @joelsmithTT let me know if this is ok for you

tenstorrent / tt-umd