SciML / OrdinaryDiffEq.jl

High performance ordinary differential equation (ODE) and differential-algebraic equation (DAE) solvers, including neural ordinary differential equations (neural ODEs) and scientific machine learning (SciML)
https://diffeq.sciml.ai/latest/
Other
521 stars 198 forks source link

Added Separate package for RKC #2253

Closed ParamThakkar123 closed 1 week ago

ParamThakkar123 commented 2 weeks ago

Checklist

Additional context

Add any other context about the problem here.

Solves a part of #2177

ParamThakkar123 commented 2 weeks ago

@ChrisRackauckas Please Review my PR for RKC refactor

oscardssmith commented 2 weeks ago

This seems to be missing the deletion of this code from OrdinaryDiffEq

ParamThakkar123 commented 2 weeks ago

@oscardssmith I will fix that and make a commit. Any more suggestions?? I will implement that too

ParamThakkar123 commented 2 weeks ago

What about rkc_tableaus.jl file ?? image

It shows that the file is too big to include

oscardssmith commented 2 weeks ago

Yeah moving that one is the main reason for this being desired.

ParamThakkar123 commented 2 weeks ago

@oscardssmith I made the changes.

ParamThakkar123 commented 2 weeks ago

@oscardssmith

ChrisRackauckas commented 2 weeks ago

This also needs to add a piece in the OrdinaryDiffEq tests that invokes the RKC tests. See how that's done in the extrapolation, since otherwise this piece is now not tested. Other than that, if this passes tests then it looks good.

ParamThakkar123 commented 2 weeks ago

@ChrisRackauckas @oscardssmith

I made the changes. But what should I do for these ? :

image image image

ChrisRackauckas commented 2 weeks ago

You were redefining abstract types for the algorithms you were importing. See the suggestions above, I merged them so CI can run.

ParamThakkar123 commented 2 weeks ago

Any more changes to be done @ChrisRackauckas ??

ChrisRackauckas commented 2 weeks ago

https://github.com/SciML/OrdinaryDiffEq.jl/actions/runs/9596502491/job/26463503251?pr=2253#step:6:653

Looks like you didn't remove the RKC's from OrdinaryDiffEq itself?

ParamThakkar123 commented 2 weeks ago

image

In the OrdinaryDiffEq.jl I removed the RKC methods and wrote them this way similar to the extrapolation one:

image

ParamThakkar123 commented 2 weeks ago

Any more changes to make ??

ParamThakkar123 commented 2 weeks ago

image

Tried to fix this but the error still occurs. I will try to figure this out.

ParamThakkar123 commented 2 weeks ago

image

Tried to fix this but the error still occurs. I will try to figure this out.

@ChrisRackauckas Everything passes the tests. Only this seems to be the problem in test failure

ParamThakkar123 commented 1 week ago

image

@ChrisRackauckas I tried to run tests on my local system. All the tests have passed for OrdinaryDiffEqStabilizedRK

ChrisRackauckas commented 1 week ago

See the code changes above. I don't think it could have worked locally on this branch without those changes because some of those are clearly missing definitions that would lead to precompilation failures.

The latest failure is now because the alg_utils.jl split is incomplete: there's still some RKC methods in that file that need to move to the sublib. That should be the last thing.

ParamThakkar123 commented 1 week ago

@ChrisRackauckas Any changes to make ??

ChrisRackauckas commented 1 week ago

https://github.com/SciML/OrdinaryDiffEq.jl/actions/runs/9624176677/job/26547530072?pr=2253#step:6:1131

https://github.com/SciML/OrdinaryDiffEq.jl/actions/runs/9624176677/job/26547530174?pr=2253#step:6:673

Yes a few things since the convergence tests when the RKC is called fail.

ParamThakkar123 commented 1 week ago

Ok. Got it.

ChrisRackauckas commented 1 week ago

Yeah no need to duplicate the Newton code. It can just be imported, and if you dev'd locally then you'll get the whole list of what to pull in from the linter.

I also did a small change to keep mirroring the OrdinaryDiffEq code of having alg_utils for the trait functions.

ParamThakkar123 commented 1 week ago

There is some problem with the utility_tests.jl, split_methods_tests.jl, waltman.jl, and jacobian.jl files this time. rkc_tests.jl shows a autodiffentiation error

ParamThakkar123 commented 1 week ago

Should I turn off automatic differentiation ??

ParamThakkar123 commented 1 week ago

image

How can I fix this ?? I was working to fix this failure

ChrisRackauckas commented 1 week ago

Don't worry about that, that seems to be due to a change in the linear algebra used for the OOP regime which makes it so it's correct to like 12 digits instead of 14, so I'm changing the tolerance on that test which was too stringent.

Consider this PR done. I'm going to do a few things to split out IRKC since it doesn't really fit with the other methods. I thought this would be an easier PR than it was because I forgot about IRKC: it's the only one that uses the Newton methods, and that is what accounts for like half of the imports 😅 . So I'll take care of that.

Let's tackle all of the explicit ones first as easy cases. Move onto doing the low storage RK, then SSPRK, then RKN, then symbolic rk. That's a good list of explicit ones. Now that you have the motion down each should only take like half an hour if you're quick and have a good local dev setup. I have a long flight to Singapore tonight so I might pick up FIRK and SDIRK.

ParamThakkar123 commented 1 week ago

Don't worry about that, that seems to be due to a change in the linear algebra used for the OOP regime which makes it so it's correct to like 12 digits instead of 14, so I'm changing the tolerance on that test which was too stringent.

Consider this PR done. I'm going to do a few things to split out IRKC since it doesn't really fit with the other methods. I thought this would be an easier PR than it was because I forgot about IRKC: it's the only one that uses the Newton methods, and that is what accounts for like half of the imports 😅 . So I'll take care of that.

Let's tackle all of the explicit ones first as easy cases. Move onto doing the low storage RK, then SSPRK, then RKN, then symbolic rk. That's a good list of explicit ones. Now that you have the motion down each should only take like half an hour if you're quick and have a good local dev setup. I have a long flight to Singapore tonight so I might pick up FIRK and SDIRK.

Ok. Sir. I am on that

ParamThakkar123 commented 1 week ago

@ChrisRackauckas I have started working on the low storage RK methods and added all the algorithms mentioned in this doc https://docs.sciml.ai/DiffEqDocs/stable/solvers/ode_solve/#Low-Storage-Methods

there are more in the explicit_rk_pde.jl, should I add them all or just the ones mentioned in the docs ??

ChrisRackauckas commented 1 week ago

Do the whole file. It's the docs that are a bit out of date

ParamThakkar123 commented 1 week ago

@ChrisRackauckas I made a separate pull request for Low Storage RK methods #2255

ParamThakkar123 commented 1 week ago

@ChrisRackauckas I started the PR on master #2256