Skip to content

[TESTS] Allow user defined output_dtype and acc_dtype in matmul tests#2769

Merged
ThomasRaoux merged 2 commits intotriton-lang:mainfrom
karupayun:matmul/add-acc-output-dtype
Dec 12, 2023
Merged

[TESTS] Allow user defined output_dtype and acc_dtype in matmul tests#2769
ThomasRaoux merged 2 commits intotriton-lang:mainfrom
karupayun:matmul/add-acc-output-dtype

Conversation

@karupayun
Copy link
Copy Markdown
Collaborator

@karupayun karupayun commented Dec 6, 2023

In this PR we are allowing to manually set acc_dtype and output_dtype in matmul test.

They are:

  • acc_dtype: So users of the test class can specify the type used internally in the dot, and not the one set by default given the two types. There are several restrictions for these types anyway.
  • output_dtype: The return type of the matmul. I included a few tests in the case of making a dot with two float16.

I also had to modify test_matmul to use a small range of values to prevent numerical issues. In the case of testing with two float16 and acc_dtype float16, since I can't force torch to use float16 internally (it uses float32), I was having precision issues when comparing the results with triton.

@karupayun karupayun requested a review from ptillet as a code owner December 6, 2023 12:29
Comment on lines +158 to +163
# Use small range of values to prevent numerical issues.
exponents = torch.randint(-4, 0, size=(m, n))
ret = (2.**exponents).to(getattr(torch, dtype)).to("cuda")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we use a smaller range only for the formats that need it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

In this PR we are allowing to manually set acc_dtype and output_dtype
in matmul test.

They are:
  - `acc_dtype`: So users of the test class can specify the type used
internally in the dot, and not the one set by default given the two
types. There are several restrictions for these types anyway.
  - `output_dtype`: The return type of the matmul. I included a few
tests in the case of making a dot with two float16.
- I had to modify test_matmul to use a small range of values to
prevent numerical issues. In the case of testing with two `float16` and
`acc_dtype` `float16`, since I can't force torch to use `float16`
internally (it uses `float32`), I was having precision issues when
comparing the results with triton.

The discussion of why we are doing this for all tests and not only for
that particular ones is simplicity, since we should not be testing
precision here:
The discussion can be seen in
openxla#6 (comment)
and openxla#6 (comment)
but I do not have a strong opinion, so I am ok with just testing with
small integers when the acc_dtype is float16.
@karupayun karupayun force-pushed the matmul/add-acc-output-dtype branch from c1563aa to c457c49 Compare December 12, 2023 18:24
@ThomasRaoux ThomasRaoux merged commit d69eb70 into triton-lang:main Dec 12, 2023
binarman pushed a commit to binarman/triton that referenced this pull request Apr 2, 2024
…triton-lang#2769)

In this PR we are allowing to manually set acc_dtype and output_dtype in
matmul test.

They are:
- `acc_dtype`: So users of the test class can specify the type used
internally in the dot, and not the one set by default given the two
types. There are several restrictions for these types anyway.
- `output_dtype`: The return type of the matmul. I included a few tests
in the case of making a dot with two float16.

I also had to modify test_matmul to use a small range of values to
prevent numerical issues. In the case of testing with two `float16` and
`acc_dtype` `float16`, since I can't force torch to use `float16`
internally (it uses `float32`), I was having precision issues when
comparing the results with triton.
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.

2 participants