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

dispatch.jl: simplify overloads for SparseMatrix * AbstractArray #181

Merged
merged 2 commits into from
Nov 27, 2023

Conversation

odow
Copy link
Member

@odow odow commented Nov 20, 2022

Part of #178

@codecov
Copy link

codecov bot commented Nov 20, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (352aa8f) 86.14% compared to head (fe5e4a1) 89.90%.
Report is 32 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #181      +/-   ##
==========================================
+ Coverage   86.14%   89.90%   +3.76%     
==========================================
  Files          20       23       +3     
  Lines        1877     2547     +670     
==========================================
+ Hits         1617     2290     +673     
+ Misses        260      257       -3     

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

@blegat
Copy link
Member

blegat commented Nov 21, 2022

Is this adding more methods that what was done previously ?

@odow
Copy link
Member Author

odow commented Nov 21, 2022

There are a few more, because it includes things like *(::Adjoint, ::Transpose). I don't really understand how we picked the current set of methods. It feels somewhat arbitrary.

@blegat
Copy link
Member

blegat commented Nov 22, 2022

I don't really understand how we picked the current set of methods. It feels somewhat arbitrary.

Trials and errors, I tried to have as few as possible but I had to add some when tests where failing.
The approach of this PR is better unless it starts adding so many methods that the compiler starts being slow or have issues.
In the long term, we should improve LinearAlgebra/SparseArrays to make this easier.

@odow odow closed this Nov 27, 2023
@odow odow reopened this Nov 27, 2023
@odow
Copy link
Member Author

odow commented Nov 27, 2023

Merging because this is a small win. I don't think we need to add all the LinearAlgebra method just yet though.

@odow odow merged commit 3a6c406 into master Nov 27, 2023
11 checks passed
@odow odow deleted the od/dispatch-3 branch November 27, 2023 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants