Skip to content

CompatHelper: bump compat for SciMLBase to 3, (keep existing compat)#2918

Open
github-actions[bot] wants to merge 1 commit intomainfrom
compathelper/new_version/2026-04-09-00-09-39-403-03213629059
Open

CompatHelper: bump compat for SciMLBase to 3, (keep existing compat)#2918
github-actions[bot] wants to merge 1 commit intomainfrom
compathelper/new_version/2026-04-09-00-09-39-403-03213629059

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented Apr 9, 2026

This pull request changes the compat entry for the SciMLBase package from 2.128 to 2.128, 3.
This keeps the compat entries for earlier versions.

Note: I have not tested your package with this new compat entry.
It is your responsibility to make sure that your package tests pass before you merge this pull request.

@ranocha ranocha force-pushed the compathelper/new_version/2026-04-09-00-09-39-403-03213629059 branch from 39b92f8 to 61dc2aa Compare April 9, 2026 00:09
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.08%. Comparing base (991f79d) to head (61dc2aa).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2918   +/-   ##
=======================================
  Coverage   97.08%   97.08%           
=======================================
  Files         612      612           
  Lines       47668    47668           
=======================================
  Hits        46277    46277           
  Misses       1391     1391           
Flag Coverage Δ
unittests 97.08% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@JoshuaLampert JoshuaLampert left a comment

Choose a reason for hiding this comment

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

Should be done in #2910.

@ChrisRackauckas-Claude
Copy link
Copy Markdown

Courtesy note from the SciMLBase v3 ecosystem sweep: this PR is a compat-only bump but Trixi has a non-trivial v3 surface area to check before merging.

22 u_modified! call sites in src/ — the hottest ones:

  • src/time_integration/methods_3Sstar.jl:223
  • src/time_integration/methods_SSP.jl:224
  • src/callbacks_step/alive.jl:101
  • src/callbacks_step/save_solution.jl:256
  • src/callbacks_step/trivial.jl:20

SciMLBase v3 renamed u_modified!derivative_discontinuity! with an @deprecate shim, so the calls still work but emit deprecation warnings, and any u_modified! method definitions on Trixi's own integrator types won't be picked up by callers who have already migrated to the new name (those callers hit the generic error("method has not been implemented for the integrator") fallback).

Suggested pattern (same one used in SciML/DiffEqCallbacks.jl#300): replace direct imports with an @static if isdefined(SciMLBase, :derivative_discontinuity!) guard so the file works under both SciMLBase v2 and v3:

@static if isdefined(SciMLBase, :derivative_discontinuity!)
    using SciMLBase: derivative_discontinuity!
else
    const derivative_discontinuity! = SciMLBase.u_modified!
end

For method definitions on Trixi integrator types, keep the existing u_modified! overload and also define derivative_discontinuity! under the isdefined guard so post-migration callers dispatch correctly.

Resolution caveat: CI will currently fail at Pkg.add on any env pulling SciMLBase 3.1 because OrdinaryDiffEq still pins RecursiveArrayTools ≤ 3.54. This will self-resolve once OrdinaryDiffEq bumps; no action needed here other than triggering a re-run afterward.

Other v3 flags in Trixi src/test came up clean: no DEProblem type refs, no 3-arg ensemble prob_func, no single-int timeseries sol[i] indexing.

cc @ChrisRackauckas

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