Skip to content

Fix SplitODEProblem regression for non-IMEX ESDIRK methods (Kvaerno3/4/5)#3653

Merged
ChrisRackauckas merged 4 commits into
SciML:masterfrom
JoshuaLampert:fix-sdirk
May 19, 2026
Merged

Fix SplitODEProblem regression for non-IMEX ESDIRK methods (Kvaerno3/4/5)#3653
ChrisRackauckas merged 4 commits into
SciML:masterfrom
JoshuaLampert:fix-sdirk

Conversation

@JoshuaLampert
Copy link
Copy Markdown
Contributor

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

After the generic IMEX tableau refactoring (#3196), Kvaerno3, Kvaerno4, and Kvaerno5 give wrong results when used with a SplitODEProblem. The non-stiff component f.f2 is silently dropped from the integration.

This is because the new _perform_step_iip! / _perform_step_oop! in generic_imex_perform_step.jl unconditionally branches on integrator.f isa SplitFunction to compute the first stage using only f.f1:

For true IMEX methods with issplit(alg) = true this is correct, but not for issplit(alg) = false as the Kvaerno methods.

This fixes the regression observed in trixi-framework/Trixi.jl#2910 (comment), @singhharsh1708.

Note that I used the help of AI tools to find the root cause, but I supervised and reviewed the changes.

@singhharsh1708
Copy link
Copy Markdown
Contributor

Verified the fix reproduces and resolves the issue locally. Kvaerno4 + SplitFunction on a simple f1=-u, f2=2u problem gives u_end=[1.738, 0.869] on master (clearly wrong — f2 dropped) and u_end≈[2.718, 1.359] after the fix (matches analytic e¹·[1.0, 0.5]). Same for Kvaerno3 and Kvaerno5, both IIP and OOP. SDIRK convergence + DAE suites still pass. Thanks for tracking it down!

Copy link
Copy Markdown
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the fix! Wold it make sense to add some CI tests to ensure that such an error does not happen again in the future?

@JoshuaLampert
Copy link
Copy Markdown
Contributor Author

Yes, makes sense. I added a test in dc27125.

@ranocha
Copy link
Copy Markdown
Member

ranocha commented May 19, 2026

Great, thanks! I assume you checked that it fails without your fixes?

@JoshuaLampert
Copy link
Copy Markdown
Contributor Author

Yes, it fails on current master with convergence rates around 0.

@ranocha
Copy link
Copy Markdown
Member

ranocha commented May 19, 2026

Thanks a lot!

@ChrisRackauckas ChrisRackauckas merged commit 192c12f into SciML:master May 19, 2026
81 of 85 checks passed
@JoshuaLampert JoshuaLampert deleted the fix-sdirk branch May 20, 2026 05:46
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.

4 participants