update precs when cache.isfresh

[oscardssmith]

successor to #528

[j-fu]

Why is this needed here ? Pl, Pr are not used by Pardiso.

[oscardssmith]

oh, is krylov the only one that uses them? I thought there was another...

[ChrisRackauckas]

IterativeSolvers, KrylovKit.

[j-fu]

Here we may need

Pl, Pr = cache.alg.precs(cache.A, cache.p)

[j-fu]

And generally, could we have it like this ?

function SciMLBase.solve!(cache::LinearCache, alg::KrylovJL; reuse_precs=false, kwargs...)
    if cache.isfresh
        if hasproperty(alg, :precs) && !isnothing(alg.precs)
            if !reuse_precs || (cache.Pl == nothing && cache.Pr == nothing)
                Pl, Pr = cache.alg.precs(cache.A, cache.p)
                cache.Pl = Pl
                cache.Pr = Pr
            end
        end
 ...

EDIT: I meant reuse_precs=false as default.

[j-fu]

Also, in the moment this leads to precs being called twice: once in init and once here.

[ChrisRackauckas]

init should set fresh to false.

[oscardssmith]

ah that's tricky. I think I will need to add an extra flag to fix this.

[oscardssmith]

@ChrisRackauckas really? I think the code relies on it being true to set up some of the C libraries.

[oscardssmith]

the problem with the current way is that it makes the fallback slow. if we didn't have the "fast path", we could set cache.isfresh=false which would recover the speed of the fast path for all types, as long as the user inits on a matrix they actually want to solve.

[ChrisRackauckas]

You could just set it to false on the slow path.

[oscardssmith]

the current approach is better by 1 factorization for users that create a LinearCache with garbage data, but those users will almost always be doing lots of factorizations, minimizing the advantage. OTOH, the current approach has overhead for users who just call solve (since the fake factorization isn't free), or users of the cache form who init on data they want to solve eventually.

[oscardssmith]

we could set it to false on the slow path, but doing so would be pretty hard since init_cacheval doesn't get the cache.

[oscardssmith]

and actually users that are initing with fake matrices can init with a 1x1 themselves, which will completely remove the penalty of my suggestion