Skip to content

Conversation

@vchuravy
Copy link
Member

@vchuravy vchuravy commented Nov 25, 2025

Partial fix to #2813

The crux is that Julia used a different function and while trsm is defined in Enzyme proper there aren't currently any rules attached https://github.com/EnzymeAD/Enzyme/blob/a13f632e2fbaed1f971de015f15e8f4b353e66cb/enzyme/Enzyme/BlasDerivatives.td#L705-L713

@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.79%. Comparing base (30e0519) to head (013ba70).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2814      +/-   ##
==========================================
- Coverage   67.79%   67.79%   -0.01%     
==========================================
  Files          58       58              
  Lines       20723    20723              
==========================================
- Hits        14050    14049       -1     
- Misses       6673     6674       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Contributor

Benchmark Results

main 013ba70... main / 013ba70...
basics/make_zero/namedtuple 0.0528 ± 0.0031 μs 0.0526 ± 0.0026 μs 1 ± 0.076
basics/make_zero/struct 0.255 ± 0.009 μs 0.253 ± 0.0057 μs 1.01 ± 0.042
basics/overhead 4.03 ± 0.001 ns 4.95 ± 0.01 ns 0.814 ± 0.0017
basics/remake_zero!/namedtuple 0.235 ± 0.008 μs 0.242 ± 0.0083 μs 0.971 ± 0.047
basics/remake_zero!/struct 0.234 ± 0.01 μs 0.233 ± 0.0084 μs 1 ± 0.056
fold_broadcast/multidim_sum_bcast/1D 10.3 ± 0.64 μs 10.3 ± 0.24 μs 1 ± 0.067
fold_broadcast/multidim_sum_bcast/2D 12.1 ± 0.29 μs 12.1 ± 0.27 μs 0.997 ± 0.033
time_to_load 1.26 ± 0.012 s 1.28 ± 0.019 s 0.987 ± 0.017

Benchmark Plots

A plot of the benchmark results has been uploaded as an artifact at https://github.com/EnzymeAD/Enzyme.jl/actions/runs/19671170824/artifacts/4673611025.

@wsmoses
Copy link
Member

wsmoses commented Nov 25, 2025

incidentally that was actually reverted: JuliaLang/LinearAlgebra.jl#1239

but apparently not backported yet

@vchuravy vchuravy merged commit 7886a73 into main Nov 25, 2025
55 of 56 checks passed
@vchuravy vchuravy deleted the vc/trsm branch November 25, 2025 18:39
"syrk",
"trmm",
"trsm",
# "trsm", Not actually implemented yet
Copy link
Member

Choose a reason for hiding this comment

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

sorry quick post commit review, can this be limited to 1.12+.

It will otherwise slow down other code by making it non-blas, if not differentiated

Copy link
Member Author

Choose a reason for hiding this comment

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

Oof, that's frustrating, shouldn't we ideally not do the blas replacement when a function is not being differentiated? Otherwise this problem occurs for many more functions?

Copy link
Member

Choose a reason for hiding this comment

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

it does indeed occur for more functions (which is why we should be somewhat sparing on the blas replacements).

The issue is that at the time of replacement, we haven't run activity analysis so we don't know what is needed and what isn't

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