-
-
Notifications
You must be signed in to change notification settings - Fork 211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use LinearSolve precs interface #2318
base: master
Are you sure you want to change the base?
Conversation
e853d84
to
791f557
Compare
Is there a link for this? I might want to do a similar cleanup of the boilerplate in NonlinearSolve |
yeah it's SciML/LinearSolve.jl#514. It's not yet fully documented but there are some docs for it and it works. |
80a5e82
to
cfa4d58
Compare
I believe this now works (once SciML/SciMLBase.jl#769 is merged so remake on the Linear solver works properly) |
Tests all pass locally (with deved SciMLBase). @ChrisRackauckas can you confirm that this is the API we want? |
1e1d569
to
25415c9
Compare
It looks right. |
hmm. The Interface II test seems to be failing stochastically. |
74afde3
to
44f8694
Compare
planning on merging as soon as tests pass |
Ah, it's not random, but it is dependent on the order of runs. Does Krylov.jl depend on the global random number generator? IMO that's pretty bad UI. |
@ChrisRackauckas This error does actually appear to be at least somewhat stochastic. Do you know where the randomness would be coming from and how to work around it? Currently, it looks like this test will fail roughly 1/60 |
Are you factorizing any uninitialized memory? The reason for the init stuff in ArrayInterface is to avoid that, so double check it is still in play. |
I'm pretty sure that's not the problem (since I'm not getting undef errors, but rather just timesteps failing repeatedly) |
You wouldn't get |
a3ff4c1
to
3368f20
Compare
It looks like the undef |
b7f0468
to
1752b50
Compare
Looks like tests are finally mostly passing |
6b3e731
to
0b0ba2e
Compare
The last failure is StabilizedIRK and I honestly have absolutely no idea why it's any different. @ChrisRackauckas any thoughts? |
fd7cb70
to
671775f
Compare
I think this is finally good to merge. |
c31fd22
to
fd35cc6
Compare
Show some before and after WPDs from the stiff ODE set. |
WPDs with or without precs? |
also why work-precision? this shouldn't change accuracy |
It should take like 10 minutes to make them and eyeball if there's a change. This is a pretty large change so it's worth double checking. |
16ba23e
to
4c53232
Compare
Here's the WP results:
|
It does look like there are some significant regressions? |
What's the current status here? |
I need to figure out what's causing this to sometimes be slower, but it's not getting merged urgently since it's not going in until OrdinaryDiffEqv7 |
This is a post OrdinaryDiffEq solver split version of #2247 (remaking was substantially easier than rebasing). It still needs a little bit of work (and for the LinearSolve side to be merged first), but it's getting pretty close.