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

Add tests for BLAS.dot, BLAS.dotc, and BLAS.dotu #842

Merged
merged 15 commits into from
Sep 10, 2023

Conversation

sethaxen
Copy link
Collaborator

As requested by @ZuseZ4, this PR just copies the tests from #739 but not the EnzymeRules.

@codecov-commenter
Copy link

codecov-commenter commented May 14, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.04% 🎉

Comparison is base (04ced8a) 78.03% compared to head (726fdc0) 78.08%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #842      +/-   ##
==========================================
+ Coverage   78.03%   78.08%   +0.04%     
==========================================
  Files          27       27              
  Lines        9148     9148              
==========================================
+ Hits         7139     7143       +4     
+ Misses       2009     2005       -4     

see 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ZuseZ4
Copy link
Member

ZuseZ4 commented May 17, 2023

Didn't intend to close, will merge it once I added coverage for dotu, dotc

@sethaxen
Copy link
Collaborator Author

I updated this PR to use EnzymeTestUtils.

@wsmoses
Copy link
Member

wsmoses commented Sep 10, 2023

sgtm and still approved to merge (once tests pass). @ZuseZ4 was also wondering if you'd be able to add tests for some other BLAS functions as well.

@sethaxen
Copy link
Collaborator Author

Sure, which ones?

@wsmoses
Copy link
Member

wsmoses commented Sep 10, 2023

@ZuseZ4 mentioned gemv/gemm/scal/axpy/spmv

also it looks like CI fails?

@ZuseZ4
Copy link
Member

ZuseZ4 commented Sep 10, 2023

Sure, which ones?

I did implement scal, axpy, gemv, spmv, gemm,
since these are the relevant ones for Julia.
So tests for these would be the most helpful for me.

@sethaxen
Copy link
Collaborator Author

also it looks like CI fails?

Pretty sure the failure is due to JuliaLang/Pkg.jl#1585. One way to workaround this is to not have a separate Project.toml in test/. But trying a different workaround.

I did implement scal, axpy, gemv, spmv, gemm,

Okay, I'll add this once I fix the CI

@wsmoses wsmoses merged commit 744ffce into EnzymeAD:main Sep 10, 2023
@ZuseZ4
Copy link
Member

ZuseZ4 commented Sep 11, 2023

thanks! Will try how tablegen does against the testcases tomorrow.

@sethaxen
Copy link
Collaborator Author

thanks! Will try how tablegen does against the testcases tomorrow.

Are the tablegen rules currently not being used? That could explain why when I added rules for scal!, I got a number of failures.

@sethaxen sethaxen deleted the blastests branch September 11, 2023 07:43
michel2323 pushed a commit that referenced this pull request Nov 7, 2023
* Add tests for BLAS rules

* Make checks approximate

* Fix approx check for tuples

* Apply suggestion

* Add EnzymeTestUtils as test dependency

* Refactor to use EnzymeTestUtils

Also removes checks for consistency with 2-arg dot and pointer tests, since these should be covered by the base case.

* Make sure dev version of EnzymeTestUtils used in testing

* Move blas.jl to test dir

* Update path

* Try to fix deving of EnzymeTestUtils

* Dev EnzymeTestUtils while running tests

* Remove EnzymeTestUtils as test dep

* Load Pkg

* Remove workaround in CI

---------

Co-authored-by: William S. Moses <[email protected]>
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.

4 participants