-
Notifications
You must be signed in to change notification settings - Fork 17
Update to use ADTypes for specifying AD backend #338
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
base: main
Are you sure you want to change the base?
Conversation
@jClugstor Thanks a lot for opening this PR! Do you need any help to get this PR forward and fix the failing tests? I don't have a good overview over the AD changes going on in OrdinaryDiffEq, but in case there are any blockers please let me know |
The tests should hopefully pass once the newest version of OrdinaryDiffEqDifferentiation comes out, which will hopefully be very soon. There are some changes in it that will break some parts of ProbNumDiffEq, so this PR should fix it. |
@jClugstor can you accept those changes? That should fix the tests here and make it mergable. |
Co-authored-by: Christopher Rackauckas <accounts@chrisrackauckas.com>
Looks like someone needs to approve the test workflows |
@jClugstor do you have any update on this PR? If there is any way I can help drive this forward let me know. |
So it looks like the failures are related to some of the ModelingToolkit indexing stuff, which I really don't know much about. I think I should be able to look in to it pretty soon though, hopefully sometime next week. |
Solve the other ones first |
The OOP tests were just a missing The AD tests have something to do with |
FYI I just changed a setting in the repo so that hopefully I won't have to approve workflows anymore (though I'll continue to check regularly to make sure that that's not a blocker). |
This should let the tests run without error at least, it just needed to use SciMLStructures to give the finite diff gradients something they can actually use. The tests still fail though, there's a bit of a difference between the output of ForwardDiff and FiniteDifferences here. I also tested FiniteDiff, and there's still a difference that's greater than the tolerance. |
Recently OrdinaryDiffEq has been upgraded to using ADTypes to specify the AD backend used in implicit solvers.
Additionally we'll be updating to use DifferentiationInterface, SciML/OrdinaryDiffEq.jl#2567.
This PR makes EK1 and EK1Diagonal use ADTypes to specify the AD backend, to make ProbNumDiffEq compatible with the upcoming versions of OrdinaryDiffEq.