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

#10226: Added or refactored BH SFPU functions #12379

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

ncvetkovicTT
Copy link
Contributor

@ncvetkovicTT ncvetkovicTT commented Sep 9, 2024

Ticket

#10226

Problem description

Some functions are not added/enabled for BH.

What's changed

Within the scope of this PR, the following changes are introduced:

  1. SFPU mask - added _int version
  2. SFPU reciprocal - modified to use submodule LLKs
  3. SFPU sqrt - modified to use submodule LLKs
  4. SFPU trunc OP - enabled for BH

Checklist

  • Post commit CI passes - #15330
  • Blackhole Post commit (if applicable) - #584
  • Model regression CI testing passes (if applicable)
  • New/Existing tests provide coverage for changes

@rtawfik01
Copy link
Contributor

Since Blackhole only has SD/FD C++ tests on pipelines, can you also run this pytest -svv tests/tt_eager/python_api_testing/sweep_tests/pytests/tt_dnn/test_eltwise_unary.py and make sure it still passes since it has not been added to CI yet?

@rtawfik01
Copy link
Contributor

Also how has the reshuffle_rows been tested?

@ncvetkovicTT ncvetkovicTT force-pushed the ncvetkovic/10226-bh_new_sfpu_functions branch from 5c5275d to 86e72c9 Compare September 9, 2024 14:52
@ncvetkovicTT
Copy link
Contributor Author

Since Blackhole only has SD/FD C++ tests on pipelines, can you also run this pytest -svv tests/tt_eager/python_api_testing/sweep_tests/pytests/tt_dnn/test_eltwise_unary.py and make sure it still passes since it has not been added to CI yet?

Tested, pass ✅

@ncvetkovicTT
Copy link
Contributor Author

ncvetkovicTT commented Sep 9, 2024

Also how has the reshuffle_rows been tested?

No, the work will be continued in https://github.com/tenstorrent/tt-metal/tree/ncvetkovic/11816-reshuffle_rows_supportas a part of #11816

@ncvetkovicTT ncvetkovicTT force-pushed the ncvetkovic/10226-bh_new_sfpu_functions branch from 406cf26 to 4ff7f1a Compare September 10, 2024 12:28
@ncvetkovicTT ncvetkovicTT force-pushed the ncvetkovic/10226-bh_new_sfpu_functions branch 3 times, most recently from 054cb49 to 4eadbc2 Compare September 10, 2024 14:08
Copy link
Contributor

@abhullar-tt abhullar-tt left a comment

Choose a reason for hiding this comment

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

I believe this should unblock some of the ops that are failing in #12349

Copy link
Contributor

@eyonland eyonland left a comment

Choose a reason for hiding this comment

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

I'd like to request that some sample of the uses cases of the sweep tests mentioned by @rtawfik01 be added to tests/ttnn/unit_tests

@abhullar-tt
Copy link
Contributor

I'd like to request that some sample of the uses cases of the sweep tests mentioned by @rtawfik01 be added to tests/ttnn/unit_tests

We are working to enable the fast-dispatch-post-commit workflow on BH CI which should cover these cases, there is a PR up where we are disabling the current failing tests on BH (see https://github.com/tenstorrent/tt-metal/tree/mchiou/11919-enable-ttnntteager-on-bh-ci)

@ncvetkovicTT ncvetkovicTT force-pushed the ncvetkovic/10226-bh_new_sfpu_functions branch from 4eadbc2 to a0d89ce Compare September 10, 2024 18:05
Copy link
Contributor

@eyonland eyonland left a comment

Choose a reason for hiding this comment

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

Based on conversation with @abhullar-tt it sounds like we have a parallel effort and we will get unit tests turned on eventually.

1. SFPU mask - added _int version
2. SFPU reciprocal - modified to use submodule LLKs
3. SFPU sqrt - modified to use submodule LLKs
4. SFPU trunc OP - enabled for BH
@ncvetkovicTT ncvetkovicTT force-pushed the ncvetkovic/10226-bh_new_sfpu_functions branch from a0d89ce to 6dd526f Compare September 11, 2024 07:16
@ncvetkovicTT ncvetkovicTT merged commit 656c659 into main Sep 11, 2024
6 checks passed
@ncvetkovicTT ncvetkovicTT deleted the ncvetkovic/10226-bh_new_sfpu_functions branch September 11, 2024 07:18
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