Skip to content

Support real and complex inputs for sparse matmul #2353

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

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

Conversation

ptiede
Copy link
Contributor

@ptiede ptiede commented Apr 11, 2025

This fixes #2321 and explicitly supports arguments that mix complex and real arrays.
The logic is annoying, so it would be good to review this since I made several choices.

Copy link
Contributor

github-actions bot commented Apr 11, 2025

Your PR requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic main) to apply these changes.

Click here to view the suggested changes.
diff --git a/src/internal_rules.jl b/src/internal_rules.jl
index c425978..5bf347a 100644
--- a/src/internal_rules.jl
+++ b/src/internal_rules.jl
@@ -912,7 +912,7 @@ function EnzymeRules.reverse(
             nothing
         end
     end
-    
+
     return (nothing, nothing, nothing, dα, dβ)
 end
 
diff --git a/test/internal_rules.jl b/test/internal_rules.jl
index 5aa25c9..5135cc2 100644
--- a/test/internal_rules.jl
+++ b/test/internal_rules.jl
@@ -727,15 +727,15 @@ end
         tout = promote_type(eltype(M), eltype(v), typeof(α), typeof(β))
         C = zeros(tout, 5)
 
-        for Tret in (Duplicated, BatchDuplicated), TM in (Const, Duplicated, BatchDuplicated), Tv in (Const, Duplicated, BatchDuplicated), 
-            Tα in (Const, Active), Tβ in (Const, Active)
+        for Tret in (Duplicated, BatchDuplicated), TM in (Const, Duplicated, BatchDuplicated), Tv in (Const, Duplicated, BatchDuplicated),
+                Tα in (Const, Active), Tβ in (Const, Active)
 
             are_activities_compatible(Tret, Tret, TM, Tv, Tα, Tβ) || continue
             test_reverse(LinearAlgebra.mul!, Tret, (C, Tret), (M, TM), (v, Tv), (α, Tα), (β, Tβ))
         end
 
-        for Tret in (Duplicated, BatchDuplicated), TM in (Const, Duplicated, BatchDuplicated), 
-            Tv in (Const, Duplicated, BatchDuplicated), bα in (true, false), bβ in (true, false)
+        for Tret in (Duplicated, BatchDuplicated), TM in (Const, Duplicated, BatchDuplicated),
+                Tv in (Const, Duplicated, BatchDuplicated), bα in (true, false), bβ in (true, false)
             are_activities_compatible(Tret, Tret, TM, Tv) || continue
             test_reverse(LinearAlgebra.mul!, Tret, (C, Tret), (M, Const), (v, Tv), (bα, Const), (bβ, Const))
         end
@@ -756,16 +756,16 @@ end
         v = randn(tv, 3, 3)
         α = rand(tα)
         β = rand(tβ)
-        
-        for Tret in (Duplicated, BatchDuplicated), TM in (Const, Duplicated, BatchDuplicated), Tv in (Const, Duplicated, BatchDuplicated), 
-            Tα in (Const, Active), Tβ in (Const, Active)
+
+        for Tret in (Duplicated, BatchDuplicated), TM in (Const, Duplicated, BatchDuplicated), Tv in (Const, Duplicated, BatchDuplicated),
+                Tα in (Const, Active), Tβ in (Const, Active)
 
             are_activities_compatible(Tret, Tret, TM, Tv, Tα, Tβ) || continue
             test_reverse(LinearAlgebra.mul!, Tret, (C, Tret), (M, TM), (v, Tv), (α, Tα), (β, Tβ))
         end
 
-        for Tret in (Duplicated, BatchDuplicated), TM in (Const, Duplicated, BatchDuplicated), 
-            Tv in (Const, Duplicated, BatchDuplicated), bα in (true, false), bβ in (true, false)
+        for Tret in (Duplicated, BatchDuplicated), TM in (Const, Duplicated, BatchDuplicated),
+                Tv in (Const, Duplicated, BatchDuplicated), bα in (true, false), bβ in (true, false)
             are_activities_compatible(Tret, Tret, TM, Tv) || continue
             test_reverse(LinearAlgebra.mul!, Tret, (C, Tret), (M, Const), (v, Tv), (bα, Const), (bβ, Const))
         end

Copy link

codecov bot commented Apr 11, 2025

Codecov Report

Attention: Patch coverage is 0% with 36 lines in your changes missing coverage. Please review.

Project coverage is 70.31%. Comparing base (037dfed) to head (b8feeb2).
Report is 365 commits behind head on main.

Files with missing lines Patch % Lines
src/internal_rules.jl 0.00% 36 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2353      +/-   ##
==========================================
+ Coverage   67.50%   70.31%   +2.81%     
==========================================
  Files          31       56      +25     
  Lines       12668    16694    +4026     
==========================================
+ Hits         8552    11739    +3187     
- Misses       4116     4955     +839     

☔ 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.

@ptiede
Copy link
Contributor Author

ptiede commented Apr 12, 2025

I don't think the test failures are from me, although it doesn't look like my tests are even getting run before Enzyme fails.
Something with the sort internal rules seems broken.

Oh shoot when I formatted something got copied incorrectly fixing now

@ptiede
Copy link
Contributor Author

ptiede commented Apr 12, 2025

@wsmoses @vchuravy I'm not sure why some of the 1.10 tests are failing, but I think this is ready for review.

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.

Enzyme Issue with Sparse Matrix and Dense Vector Multiplication
1 participant