Skip to content

Comments

Buffered matrix multiplication#83

Merged
blegat merged 2 commits intomasterfrom
bl/buff_matmul
Apr 8, 2021
Merged

Buffered matrix multiplication#83
blegat merged 2 commits intomasterfrom
bl/buff_matmul

Conversation

@blegat
Copy link
Member

@blegat blegat commented Apr 1, 2021 •
edited
Loading

@blegat blegat mentioned this pull request Apr 1, 2021
Merged
@codecov
Copy link

codecov bot commented Apr 1, 2021 •
edited
Loading

Codecov Report

Merging #83 (bb96a3f) into master (595460d) will decrease coverage by 0.16%.
The diff coverage is 92.59%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #83      +/-   ##
==========================================
- Coverage   85.38%   85.22%   -0.17%     
==========================================
  Files          19       18       -1     
  Lines        1355     1509     +154     
==========================================
+ Hits         1157     1286     +129     
- Misses        198      223      +25
Impacted Files Coverage Δ
src/interface.jl 96.38% <87.50%> (+2.38%) ⬆️
src/linear_algebra.jl 96.45% <100.00%> (+0.87%) ⬆️
src/rewrite.jl 76.92% <0.00%> (-13.67%) ⬇️
src/bigint.jl 100.00% <0.00%> (ø)
src/bigfloat.jl 100.00% <0.00%> (ø)
src/Test/sparse.jl 100.00% <0.00%> (ø)
src/Test/Test.jl
src/Test/array.jl 99.63% <0.00%> (+0.03%) ⬆️
src/Test/quadratic.jl 97.70% <0.00%> (+0.07%) ⬆️
src/Test/scalar.jl 94.20% <0.00%> (+0.55%) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 595460d...bb96a3f. Read the comment docs.

odow
odow reviewed Apr 1, 2021
View reviewed changes
src/linear_algebra.jl Outdated
_dim_check(C, A, B)
_add_mul_array(buffer, C, A, B)
end
# Break ambiguity
Copy link
Member

@odow odow Apr 1, 2021

Choose a reason for hiding this comment

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

Which ambiguity? It seems weird to have to define the Nothing version when it is identical.

Copy link
Member Author

@blegat blegat Apr 3, 2021

Choose a reason for hiding this comment

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

With
https://github.com/jump-dev/MutableArithmetics.jl/blob/master/src/interface.jl#L289-L291
We could make a fallback to avoid ambiguity

@blegat blegat requested a review from odow April 8, 2021 13:10
odow
odow approved these changes Apr 8, 2021
View reviewed changes
Copy link
Member

@odow odow left a comment

Choose a reason for hiding this comment

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

At some point, we're going to need to write really good docs for all the different ways mutability is implemented.

@blegat blegat merged commit 234cf55 into master Apr 8, 2021
@odow odow deleted the bl/buff_matmul branch October 17, 2021 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@odow odow odow approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@blegat @odow