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

Simplify tests. #124

Merged
merged 1 commit into from
Jul 2, 2023
Merged

Simplify tests. #124

merged 1 commit into from
Jul 2, 2023

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented Jun 30, 2023

  • use @test + for automatic isapprox
  • use mul! directly in order to perform a mixed-mode multiplication on the CPU too

That last point slows down the tests a bit, but I think it's a better comparison. @thomasfaingnaert, was there a reason you compared against multiplications that were done in the output type instead of the input type? Maybe we should just scale down the very big (2048x2048x2048) tests.

@maleadt
Copy link
Member Author

maleadt commented Jun 30, 2023

Benchmark results for commit 6abd7c1 (comparing to fa335a1):

ID before after change
["wmma", "Float16*Float16=Float32 (N=16384, A=N, B=T)"] 324.529 ms ± 7.443 ms 334.610 ms ± 1.392 ms -6.1% ❌

@codecov
Copy link

codecov bot commented Jun 30, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (a7f6021) 30.35% compared to head (6abd7c1) 30.35%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #124   +/-   ##
=======================================
  Coverage   30.35%   30.35%           
=======================================
  Files          11       11           
  Lines         784      784           
=======================================
  Hits          238      238           
  Misses        546      546           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@thomasfaingnaert
Copy link
Member

thomasfaingnaert commented Jul 1, 2023

@thomasfaingnaert, was there a reason you compared against multiplications that were done in the output type instead of the input type?

Not sure. I don't remember any particular reason, in any case.

@maleadt maleadt merged commit 781f1de into master Jul 2, 2023
@maleadt maleadt deleted the tb/simplify_tests branch July 2, 2023 07:09
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