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.

Sign up for GitHub

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

Jump to bottom

Fast path for Two Point BVPs #109

Merged
merged 15 commits into from
Sep 30, 2023
Merged

Fast path for Two Point BVPs #109

merged 15 commits into from
Sep 30, 2023

Conversation

avik-pal
Copy link
Member

@avik-pal avik-pal commented Sep 12, 2023 •
edited
Loading

  • Tests from ODEInterface.jl

@avik-pal avik-pal mentioned this pull request Sep 13, 2023
Merged
11 tasks
@ChrisRackauckas
Copy link
Member

Looks reasonable but tests fail?

@avik-pal
Copy link
Member Author

Looks reasonable but tests fail?

This needs to wait on SciML/SciMLBase.jl#477

I will prototype a version using ODEInterface.jl. We should be ready to merge the SciMLBase one once that is done.

@avik-pal avik-pal mentioned this pull request Sep 20, 2023
Merged
@avik-pal avik-pal force-pushed the ap/twopoint branch 3 times, most recently from 3571258 to 0f4b1df Compare September 23, 2023 18:28
@avik-pal
Copy link
Member Author

This will have to wait till we release the NonlinearSolve 2.0. The released versions can't handle residual prototype it seems so they wont work for two point BVPs.

@ChrisRackauckas ChrisRackauckas mentioned this pull request Sep 30, 2023
Merged
ChrisRackauckas
ChrisRackauckas reviewed Sep 30, 2023
View reviewed changes
src/algorithms.jl
Comment on lines -1 to +2
const DEFAULT_NLSOLVE_SHOOTING = TrustRegion(; autodiff = Val(true))
const DEFAULT_NLSOLVE_MIRK = NewtonRaphson(; autodiff = Val(true))
const DEFAULT_NLSOLVE_SHOOTING = NewtonRaphson()
const DEFAULT_NLSOLVE_MIRK = NewtonRaphson()
Copy link
Member

Choose a reason for hiding this comment

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

Not so sure about that.

Copy link
Member Author

Choose a reason for hiding this comment

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

This change to NewtonRaphson was before the TR bugs were fixed. We can revert this part. The autodiff = true is redundant wince that is the current default.

@ChrisRackauckas
Copy link
Member

Is there a reason colnew isn't supported here? Have you checked its interface to see that it'll be fine?

@ChrisRackauckas ChrisRackauckas merged commit 9a97340 into SciML:master Sep 30, 2023
7 of 8 checks passed
@avik-pal avik-pal deleted the ap/twopoint branch September 30, 2023 15:15
@avik-pal
Copy link
Member Author

colnew needs some manipulation on our end to match the interface. But we don't need to add anything additional on SciMLBase side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrisRackauckas ChrisRackauckas ChrisRackauckas left review comments

@ErikQQY ErikQQY Awaiting requested review from ErikQQY

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@avik-pal @ChrisRackauckas