update values before klu_factor

[ChrisRackauckas]

Otherwise it's just using the values from before!

BTW, why do we need to klu_factor before klu!, instead of just klu!? Don't that do the factorization as well?

Fixes SciML/OrdinaryDiffEq.jl#1701

[ChrisRackauckas]

@Wimmerer do you know why after that change we'd suddenly get https://github.com/SciML/ModelingToolkit.jl/runs/7370823848?check_suite_focus=true#step:6:739 ?

#156

It was never dispatching to that solve before.

[rayegun]

I'm going to have to step through this code to be sure this is what we want. The refactor occurs in this line:

LinearSolve.jl/src/factorization.jl

Line 150 in cd582d9

fact = KLU.klu!(cache.cacheval, A)

, which is also where fact.nzval is updated to be the same as A.nzval.

Why exactly are we hitting the singular error? Is A being changed between init_cacheval and solve?

First thing I want to try here is actually running klu in init_cacheval rather than just constructing the factorization. I'll make a PR to do that. That way we know we have a factorization to refactor with rather than relying on internal fields to check for that.

The KLU refactor method also needs several exceptions checked and handled gracefully now that I look at it.

[rayegun]

For UMFPACK I recall there were some changes recently which fixed some things in UMFPACK refactors. We may just disable resolves temporarily until 1.9 if I'm correct. I'll try to figure out a way to test that manually.

[rayegun]

Or rather, the question is: is A being created at init_cacheval time with a singular matrix, then set_A is being used to make the actual matrix?

[codecov]

Codecov Report

Merging #157 (cd582d9) into main (c5168f3) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #157      +/-   ##
==========================================
+ Coverage   65.66%   65.71%   +0.05%     
==========================================
  Files           9        9              
  Lines         632      633       +1     
==========================================
+ Hits          415      416       +1     
  Misses        217      217              

Impacted Files	Coverage Δ
src/factorization.jl	77.35% <100.00%> (+0.10%)	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[ChrisRackauckas]

Or rather, the question is: is A being created at init_cacheval time with a singular matrix, then set_A is being used to make the actual matrix?

Yes, it's creating in OrdinaryDiffEq.jl with a zero matrix matching the same sparsity pattern at the initialization time, and then when solving it updates the value to the Jacobian and factorizes. So I think #158 would error (and we should try to capture this in tests).

The refactor occurs in this line:

Then what does klu_factor do? Sounds like we can just cut that line and directly go to the klu! call?

[ChrisRackauckas]

I'm going to merge this so downstream is fixed, but we should continue to discuss.

[rayegun]

Okay that's the issue then. In order to refactor KLU requires an existing factorization (unlike UMFPACK AFAIK). So we need to detect that, or set some flag to ignore the first matrix.

Or perhaps klu!(fact, A) should solve with A even if fact wasn't solved originally.