#12883: Add initial unit tests for N300 #12922
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Aswinmcw
requested review from
eyonland,
patrickroberts,
yan-zaretskiy,
cfjchu,
xanderchin,
TT-BrianLiu,
ayerofieiev-tt,
dmakoviichuk-tt,
razorback3,
dongjin-na and
SeanNijjar
as code owners
Aswinmcw
force-pushed
the
Aswinmcw/n300_unit_tests
branch
2 times, most recently
from
0e64ab0
to
6ab93d0
Compare
Aswinmcw
force-pushed
the
Aswinmcw/n300_unit_tests
branch
from
6ab93d0
to
facc1e1
Compare
SeanNijjar
requested changes
Sep 20, 2024
| from models.utility_functions import skip_for_grayskull | ||
|
|
||
|
|
||
| def is_unsupported_case(input_shape, dim, mem_config, num_devices, num_links, input_dtype, layout): |
There was a problem hiding this comment.
This looks like it's still the same check as in test_all_gather.py. Please import and use that one instead.
| return False, "" | ||
|
|
||
|
|
||
| def run_all_gather_on_n300_impl( |
There was a problem hiding this comment.
Can we commonize this with the main run function from test all-gather? Really the only difference should be the device count check - everything else I believe should be commonizable.
| if len(all_devices) != 2: | ||
| pytest.skip("Not N300!") | ||
|
|
||
| for device in all_devices: |
There was a problem hiding this comment.
Same commonization comment
| if (this->receiver_device_id == this->sender_device_id) { | ||
| TT_FATAL(input_tensor.device()->get_ethernet_sockets(this->receiver_device_id.value()).size() >= 2 * this->num_links, "2 Device all gather requires at least 2 eth connections per link"); | ||
| } else { | ||
| if (this->receiver_device_id != this->sender_device_id) { |
There was a problem hiding this comment.
We should just be able to delete the condition. For 2 chip, we should always have topology linear anyways, so the checks inside should always pass.
Aswinmcw
force-pushed
the
Aswinmcw/n300_unit_tests
branch
2 times, most recently
from
e50491a
to
3690932
Compare
Aswinmcw
force-pushed
the
Aswinmcw/n300_unit_tests
branch
from
8640b3c
to
022a751
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tracking Issue : #12883
Currently we don't have any tests for CCL to be run in N300, this PR will add initial tests to be run as post commit
Checklist