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

Fix fused_map_reduce when result is a matrix #319

Closed
wants to merge 3 commits into from
Closed

Conversation

blegat
Copy link
Member

@blegat blegat commented Jan 13, 2025

This changes the behavior because now the dot product of empty vectors is MA.Zero() :/
On the other hand, fused_map_reduce(::typeof(add_mul), ::Vector{Int}, ::Vector{Matrix{Int}}) cannot return anything else than MA.Zero() in case the vectors are empty.

This is now non-breaking. Note that fused_map_reduce(add_mul, a, b) gets called if the user does a' * b and if eltype(a) is a scalar and eltype(b) is an array then it's currently failing. This PR fixes this use case.

See also JuliaAlgebra/StarAlgebras.jl#40

Closes #318

Copy link

codecov bot commented Jan 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.67%. Comparing base (006efd4) to head (211701b).
Report is 18 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #319      +/-   ##
==========================================
- Coverage   91.83%   91.67%   -0.16%     
==========================================
  Files          23       23              
  Lines        2205     2259      +54     
==========================================
+ Hits         2025     2071      +46     
- Misses        180      188       +8     

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

@blegat
Copy link
Member Author

blegat commented Jan 13, 2025

This triggers a regression on Julia v1.6. We could start playing around adding type asserts like ::Union{Zero,T} but we could also just drop Julia v1.6

@odow
Copy link
Member

odow commented Jan 13, 2025

Closing in favor of #320

@odow odow closed this Jan 13, 2025
@odow odow deleted the bl/fuse_matrix branch January 13, 2025 22:50
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.

dot product fails when the result is a matrix
2 participants