search

tenstorrent / tt-metal

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

ttnn.embedding_bw inconsistent documentation regarding input tensors dtypes #13685

Open amalbasaTT opened 1 week ago

amalbasaTT commented 1 week ago

ttnn.embedding_bw doesn't support output and input gradient tensors having different dtypes. This bug/constraint should be put in the documentation. . To Reproduce Steps to reproduce the behavior: Sweep test for embedding_bw is located in 'tests/sweep_framework/sweeps/eltwise/binary_backward/embedding_bw/embedding_bw.py'

  1. Checkout branch amalbasaTT/backward_ops-sweeps-2 (soon to be merged to main) 
    git checkout amalbasaTT/backward_ops-sweeps-2
  2. Go to 'tests/sweep_framework/sweeps/eltwise/binary_backward/embedding_bw/embedding_bw.py'
  3. Remove invalidate_vector function
  4. Generate new parameter vectors and run the sweep test 
    python3 tests/sweep_framework/parameter_generator.py --elastic cloud --module-name eltwise.binary_backward.embedding_bw.embedding_bw
python3 tests/sweep_framework/runner.py --elastic cloud --module-name eltwise.binary_backward.embedding_bw.embedding_bw
  5. See the error. Results can be found on elastic cloud as explained here: https://github.com/tenstorrent/tt-metal/tree/main/tests/sweep_framework

Expected behavior For vectors where output_dtype and grad_dtype are different, test will fail with the message "Output and input gradient tensors must have the same dtype"

amahmudTT commented 3 days ago

@amalbasaTT For this bug do you mean that we are not supposed to allow different data types for output and input gradient tensors and we should exit with an error message ? Or do you mean different data types should be able to work together ?

amalbasaTT commented 2 days ago

@amalbasaTT For this bug do you mean that we are not supposed to allow different data types for output and input gradient tensors and we should exit with an error message ? Or do you mean different data types should be able to work together ?

I mean that this constraint should be in the documentation. And I think that different data types should be supported. I fixed the issue text so that the description of the problem isn't so vague, my bad on that.