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

Test more BLAS functions #1046

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Test more BLAS functions #1046

wants to merge 18 commits into from

Conversation

sethaxen
Copy link
Collaborator

@sethaxen sethaxen commented Sep 11, 2023

This PR adds tests using EnzymeTestUtils for

  • BLAS.scal!
  • BLAS.axpy!
  • BLAS.gemv!
  • BLAS.spmv!
  • BLAS.gemm!

@ZuseZ4
Copy link
Member

ZuseZ4 commented Sep 11, 2023

Answering your question from your old PR, I have not enabled the tablegen rules by default yet.
Mainly I was not convinced that the tests I wrote were enough, which is why I was looking for this larger testset.
We have a flag which allows to enable tablegen though:
Enzyme.Compiler.bitcode_replacement!(false)
But if this testset pass for the fallback then I can work myself on fixing remaining tablegen failures and turn it on afterwards.

Here is the related code:

disableFallback = String[]

@sethaxen sethaxen marked this pull request as ready for review September 12, 2023 21:59
@wsmoses
Copy link
Member

wsmoses commented Sep 12, 2023

@sethaxen can you mark the currently failing tests as skipped (and open corresponding issues)

@wsmoses
Copy link
Member

wsmoses commented Sep 13, 2023

@sethaxen 1.6 fails for unrelated reasons.

All of the "local" ones have the fix (aka it requires a new jll with the fix, which we haven't yet released). That fixed your issue for me.

1.7+ have different GC assertion errors. If you can isolate the cause, I can try to include a fix in the same jll release.

@wsmoses
Copy link
Member

wsmoses commented Sep 14, 2023

jll bump has landed, so updating this, CI should have that fix. The remaining failures still need investigation if you're able @sethaxen

@sethaxen
Copy link
Collaborator Author

Thanks! I'll check later. For some reason dotu and dotc started failing and not I think from changes I made. Still trying to isolate the cause.

@wsmoses
Copy link
Member

wsmoses commented Sep 24, 2023

bumping @sethaxen if you've had a chance to isolate?

@sethaxen
Copy link
Collaborator Author

bumping @sethaxen if you've had a chance to isolate?

No, this turned out to be much more time-consuming than anticipated, and I don't think I'll be able to finish it soon. It should be fairly straightforward to finish if someone else wants to take it on, just slow to identify which tests will cause Julia to crash, which need to be marked broken, and what the MWE is for an issue.

@ZuseZ4
Copy link
Member

ZuseZ4 commented Nov 28, 2023

julia: /buildworker/worker/package_linux64/build/src/llvm-late-gc-lowering.cpp:855: std::vector<int, std::allocator<int> > LateLowerGCFrame::NumberAllBase(State&, llvm::Value*): Assertion `Tracked.size() == Numbers.size()' failed.

Julia getting incorrect DiffeUse info, causing gc to free too early? Or is that something else?

@wsmoses
Copy link
Member

wsmoses commented Nov 28, 2023

GC related for sure, how or what causes is unclear

Copy link
Contributor

github-actions bot commented Jul 14, 2024

Benchmark Results

main fcfa7f3... main/fcfa7f3295e4b7...
basics/overhead 4.34 ± 0.01 ns 4.03 ± 0.01 ns 1.08
time_to_load 0.472 ± 0.0051 s 0.494 ± 0.02 s 0.957

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

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.

3 participants