tenstorrent / tt-metal

:metal: TT-NN operator library, and TT-Metalium low level kernel programming model.
Apache License 2.0
442 stars 63 forks source link

ttnn.signbit operation failed with low PCC [Grayskull and Wormhole] #7749

Open banekg opened 5 months ago

banekg commented 5 months ago

Describe the bug ttnn.signbit operation breaks with "low PCC value" error in some test cases.

To Reproduce Steps to reproduce the behavior:

  1. Checkout barsic/ttnn-param-fix-skipped branch. To be merged into main soon.
  2. Run unit test test_signbit.py by using this command: pytest tests/ttnn/python_api_testing/non_working_unit_tests/grayskull/test_signbit.py

Expected behavior There is a test case presented in the unit test tests/ttnn/python_api_testing/non_working_unit_tests/grayskull/test_signbit.py and it is expected to fail with Equal check failed error.

Getting Additional info for the operation under test and its behavior To get additional information and results for different combinations of input shapes, types, layouts and memory configs for which this operation was tested you can also run locally sweeps for ttnn.std and check the results. To do this you should:

  1. Follow the Getting Started page to setup the repo, environment variables and python-env
  2. Activate source build/python_env/bin/activate
  3. Run sweeps by using python tests/tt_eager/python_api_testing/sweep_tests/run_pytorch_test.py -i tests/ttnn/python_api_testing/sweep_tests/test_configs/ci_sweep_tests_broken/grayskull/ttnn_eltwise_signbit_test.yaml -o ./result-sweeps
  4. After the run is completed all test sweeps results should be available inside specified output directory (in this case ./result-sweeps). There you will find signbit_sweep.csv which holds all executed sweeps, among which you can also find the ones that failed and were recreated by the unit test, which you can get by searching unique data_seed field.
umadevimcw commented 5 months ago

@jliangTT This is already discussed and blocked as we are not able to store the -0.0

bharane-ab commented 5 months ago

@jliangTT This is already discussed and blocked as we are not able to store the -0.0

As mentioned above we are not able to store-0.0 so signbit is fixed for positive 0.0 in PR link

jliangTT commented 5 months ago

Yes, spun up a high BW discussion with the leads here: https://tenstorrent.slack.com/archives/C05GH08C58F/p1714406021866519 (internal)

Let's continue to mark those as blocked in the spreadsheet and focus on other stuff. thanks!

eyonland commented 4 months ago

Please close this once the tests have been updated. See #8944

VirdhatchaniKN commented 2 days ago

Hi @ttmtrajkovic , @tt-aho , @banekg , @rtawfik01 We have a bug issue related to signbit and we have a few questions here :

Would like your inputs on this to proceed further Thanks

VirdhatchaniKN commented 2 days ago

After discussions with @ttmtrajkovic , Issue Assigned to LLK Team https://github.com/tenstorrent/tt-metal/issues/13953

Moving this issue to On-Hold status