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

Upgrade path for check_error interface conformity #897

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

termi-official
Copy link
Contributor

Precedes #896 to provide an upgrade path for last_step_failed

@ChrisRackauckas
Copy link
Member

What exactly is this for?

@termi-official
Copy link
Contributor Author

This is the first step for #896 (see #896 (comment)). With this change I can as next step go to the downstream packages and adjust the last_step_failed function, and then I can go back to 896. (If we just merge 869 almost all CIs fail).

The larger context is to simplify the development of custom time integrators compatible which are at least partially compatible with the SciML ecosystem.

@ChrisRackauckas
Copy link
Member

If the steps failed and the Newton algorithm diverged, I think we'd still error out unless the system had a bool set to just continue?

@termi-official
Copy link
Contributor Author

termi-official commented Jan 22, 2025

Right. However, this error is thrown handled in a different code path. This PR merely hoists the condition out of the function call. See e.g. https://github.com/SciML/OrdinaryDiffEq.jl/blob/013c00d81292d7ddce87841b7765dde030c2875e/lib/OrdinaryDiffEqCore/src/integrators/integrator_utils.jl#L34-L36 . This function is copy pasted across the all packages with custom integrators in SciML org.

@termi-official
Copy link
Contributor Author

If the steps failed and the Newton algorithm diverged, I think we'd still error out unless the system had a bool set to just continue?

Reading over it again, maybe I misunderstood you here. Right now in OrdinaryDiffEqCore the integrator has do_error_check, which is set to true in each iteration header. I cannot find the mentioned code path in your comment. I.e. I do not see how the Newton convergence failure for adaptive time integration algorithms results in the ConvergenceFailure flag being set for the integrator.

https://github.com/search?q=org%3ASciML%20ConvergenceFailure&type=code

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.

2 participants