SciML / LinearSolve.jl

LinearSolve.jl: High-Performance Unified Interface for Linear Solvers in Julia. Easily switch between factorization and Krylov methods, add preconditioners, and all in one interface.
https://docs.sciml.ai/LinearSolve/stable/
Other
241 stars 52 forks source link

Proper handling of static arrays #444

Closed avik-pal closed 8 months ago

codecov[bot] commented 8 months ago

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (bda160c) 65.29% compared to head (a31f99f) 65.36%.

Files Patch % Lines
src/common.jl 89.28% 3 Missing :warning:
src/default.jl 80.00% 1 Missing :warning:
src/factorization.jl 88.88% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #444 +/- ## ========================================== + Coverage 65.29% 65.36% +0.06% ========================================== Files 27 27 Lines 2072 2096 +24 ========================================== + Hits 1353 1370 +17 - Misses 719 726 +7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ChrisRackauckas commented 8 months ago

Could we add an inference and no alloc check on static array usage too?

avik-pal commented 8 months ago

There is this 1 allocation while creating the LinearCache which I can't seem to track image

ChrisRackauckas commented 8 months ago

This is kind of the same thing as the SimpleNonlinearSolve/SimpleDiffEq case. If someone calls solve and not init, then with static things doing something simpler is advantageous. I would think that if someone calls solve on a StaticArray we should basically just do A\b and bypass the rest of LinearSolve.jl. I did a bunch of testing when making https://github.com/SciML/OrdinaryDiffEq.jl/blob/master/src/derivative_utils.jl#L1-L19 and for sufficiently small problems caching the LU with a static array (or the inv) is simply not helpful (track down the PR with the old benchmarks). This could be size dependent too.

Now if they do init I think this all makes sense though, just solve should have a fast path.

avik-pal commented 8 months ago

Yeah that makes sense. Though I feel even for init with SArrays we shouldn't really cache anything, just pretend to cache it. I have found operations mixing MArrays and SArrays to be slower than just doing the uncached SArrays

ChrisRackauckas commented 8 months ago

Agreed