Skip to content

Conversation

c42f
Copy link
Member

@c42f c42f commented May 15, 2019

Fix #606

@c42f c42f mentioned this pull request May 15, 2019
@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 81.704% when pulling 5415ea5 on cjf/fix-nonsquare-backdivide-rhs into ee01ea1 on master.

@@ -1,17 +1,17 @@
@inline (\)(a::StaticMatrix, b::StaticVecOrMat) = solve(Size(a), Size(b), a, b)
@inline (\)(a::StaticMatrix, b::StaticVecOrMat) = _solve(Size(a), Size(b), a, b)
Copy link
Member Author

Choose a reason for hiding this comment

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

By the way, I've renamed solve to _solve here just to make extra clear that it's an internal function.

@codecov-io
Copy link

codecov-io commented May 15, 2019

Codecov Report

Merging #607 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #607      +/-   ##
=========================================
+ Coverage   81.69%   81.7%   +<.01%     
=========================================
  Files          39      39              
  Lines        2781    2782       +1     
=========================================
+ Hits         2272    2273       +1     
  Misses        509     509
Impacted Files Coverage Δ
src/solve.jl 100% <100%> (ø) ⬆️

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 ee01ea1...5415ea5. Read the comment docs.

@c42f
Copy link
Member Author

c42f commented May 16, 2019

I think this should be pretty safe so I'll merge it shortly.

However, always happy for a review. Any takers? @jw3126?

c = similar(b, T)
for col = 1:Sb[2]
@inbounds c[:, col] = solve(Size($Sa), Size($Sa[1],), a, b[:, col])
@inbounds c[:, col] = _solve(Size($Sa), Size($Sa[1],), a, b[:, col])
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure here. Does b[: col] allocate if b is a MArray?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this is one of the luxuries of the julia-1.0 compiler: a temporary MArray which doesn't escape the function doesn't result in any allocations. I still can't quite believe it myself :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! LGTM

@c42f c42f merged commit 3904426 into master May 17, 2019
@c42f c42f deleted the cjf/fix-nonsquare-backdivide-rhs branch May 17, 2019 00:02
@c42f c42f mentioned this pull request Jun 6, 2019
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.

Missing \ methods

5 participants