Skip to content
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

[WIP] Update to Sundials 5 #235

Merged
merged 129 commits into from
Apr 5, 2020
Merged

[WIP] Update to Sundials 5 #235

merged 129 commits into from
Apr 5, 2020

Conversation

ChrisRackauckas
Copy link
Member

Opening the PR to track it.

@jd-lara where is this at right now? Need help with anything?

Project.toml Outdated Show resolved Hide resolved
src/Sundials.jl Outdated Show resolved Hide resolved
Project.toml Outdated Show resolved Hide resolved
@rodrigomha
Copy link
Contributor

Opening the PR to track it.

@jd-lara where is this at right now? Need help with anything?

Hi Chris. I'm also working with @jd-lara in the update. We are still working on the wrapped api and will have to go the additional constants and types that were added on Sundials 5. For now we don't need help, but probably will need some help later in testing new features, like selecting non linear solvers or new parameters.

@jd-lara
Copy link
Contributor

jd-lara commented Apr 3, 2020

@ChrisRackauckas and @YingboMa The PR is ready for review. Tests are passing and ERK is enabled. MRI is still work in progress but I don't think that should be blocked since it wasn't part of Sundials 3. We will keep working on it ask the Sundials guys about it.

test/Project.toml Outdated Show resolved Hide resolved
struct PTFQMR <: SundialsLinearSolver end
struct KLU <: SundialsLinearSolver end
struct LapackBand <: SundialsLinearSolver end
struct LapackDense <: SundialsLinearSolver end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to default to the Lapack versions. Also, let's make sure the docs get updated to add these.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an action item for me?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action item for @YingboMa to be aware of.

@ChrisRackauckas
Copy link
Member Author

To be clear, the plan will be to bring this into master, but no tag or release will be done until the common interface is fixed and worked. The tag will obviously be v4.0. Sundials split and renamed ARKODE -> ARKstep and ERKstep, and so we can do a deprecation along those lines too, but should be a proper deprecation so that users get auto-forwarded. For the v4.0 release, I am going to say that let's not worry about KLU for now, but that should be on our minds to get fixed up.

Courtesy ping to @MartinOtter on this.

@ChrisRackauckas
Copy link
Member Author

Looks like the review only found a bunch of small things, so we can get that quickly cleaned up and merged.

@jd-lara
Copy link
Contributor

jd-lara commented Apr 4, 2020

Looks like the review only found a bunch of small things, so we can get that quickly cleaned up and merged.

I will take care of the comments today.

@YingboMa
Copy link
Member

YingboMa commented Apr 5, 2020

I committed updates to the common interface here https://github.com/SciML/Sundials.jl/commits/jd/sundials5

@ChrisRackauckas
Copy link
Member Author

Comments were taken care of, issues were opened for what is left, so I will merge when tests pass and then Yingbo you should rebase and PR to master.

@jd-lara
Copy link
Contributor

jd-lara commented Apr 5, 2020

To be clear, the plan will be to bring this into master, but no tag or release will be done until the common interface is fixed and worked. The tag will obviously be v4.0. Sundials split and renamed ARKODE -> ARKstep and ERKstep, and so we can do a deprecation along those lines too, but should be a proper deprecation so that users get auto-forwarded. For the v4.0 release, I am going to say that let's not worry about KLU for now, but that should be on our minds to get fixed up.

Courtesy ping to @MartinOtter on this.

Just to add to this, there is a new method for multi-rate ODE's (MRI) that are still trying to implement. There are some kinks with the API but should be a new integrator and structure.

@YingboMa YingboMa merged commit 35911c8 into SciML:master Apr 5, 2020
@ChrisRackauckas
Copy link
Member Author

Cheers. Great work @jd-lara . I'll invite you to the Sundials team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants