Skip to content
This repository was archived by the owner on Mar 12, 2021. It is now read-only.

Upstream ldiv! overload #580

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ChrisRackauckas
Copy link
Member

DiffEqBase.jl has been carrying an ldiv! overload to make it work for awhile (https://github.com/JuliaDiffEq/DiffEqBase.jl/blob/master/src/init.jl#L148-L152), and I think it might be a good time to upstream it.

DiffEqBase.jl has been carrying an ldiv! overload to make it work for awhile (https://github.com/JuliaDiffEq/DiffEqBase.jl/blob/master/src/init.jl#L148-L152), and I think it might be a good time to upstream it.
@maleadt
Copy link
Member

maleadt commented Jan 31, 2020

Thanks! Want to add a test?

@ChrisRackauckas
Copy link
Member Author

test added

@maleadt
Copy link
Member

maleadt commented Feb 4, 2020

bors r+

bors bot added a commit that referenced this pull request Feb 4, 2020
580: Upstream ldiv! overload r=maleadt a=ChrisRackauckas

DiffEqBase.jl has been carrying an ldiv! overload to make it work for awhile (https://github.com/JuliaDiffEq/DiffEqBase.jl/blob/master/src/init.jl#L148-L152), and I think it might be a good time to upstream it.

Co-authored-by: Christopher Rackauckas <[email protected]>
@bors
Copy link
Contributor

bors bot commented Feb 4, 2020

Build failed

@maleadt
Copy link
Member

maleadt commented Feb 11, 2020

@ChrisRackauckas The CI failure looks related?

@xukai92
Copy link
Contributor

xukai92 commented Feb 11, 2020

Would it make sense to make https://github.com/JuliaGPU/CuArrays.jl/blob/master/src/linalg.jl take use of this?

@xukai92
Copy link
Contributor

xukai92 commented Feb 12, 2020

Emmm. Together with qr!, this implementation seems to be slower than what we have in https://github.com/JuliaGPU/CuArrays.jl/blob/master/src/linalg.jl#L9-L13

@ChrisRackauckas
Copy link
Member Author

But that implementation cannot reuse factorizations, so this is a lot faster in practice.

@xukai92
Copy link
Contributor

xukai92 commented Feb 12, 2020

Good point!

@xukai92
Copy link
Contributor

xukai92 commented Feb 12, 2020

I guess we should keep both then. I use \ in a case where no reuse could be taken.

@ChrisRackauckas
Copy link
Member Author

It's also possible there's a better way to do this, but I haven't found it 🤷‍♂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants