Repository navigation
Declare the threading options public so OrdinaryDiffEq.PolyesterThreads() resolves - #4196
Merged
ChrisRackauckas merged 1 commit intoAug 9, 2026
Conversation
…ads()` resolves `Sequential`, `BaseThreads` and `PolyesterThreads` are public API of OrdinaryDiffEqCore, but in v7 nothing downstream carried them, so the v6 spelling `OrdinaryDiffEq.PolyesterThreads()` stopped resolving. Import them in OrdinaryDiffEq and in the four sublibraries whose solvers take `threading` (Extrapolation, FIRK, PDIRK, PRK), and declare them `public` there. The sublibraries matter because the umbrella exports only six algorithms, so an extrapolation user reaches the solver through `using OrdinaryDiffEqExtrapolation`. `public` rather than `export`: bare `PolyesterThreads()` never worked in v6 either — v6's own test suite qualified it, and no OrdinaryDiffEqCore release has ever exported these. What v7 broke is the qualified spelling, so `public` restores exactly that without pushing a name as generic as `Sequential` into every user's namespace. Bare use stays an `UndefVarError`, and a test asserts it stays one. Core's source is unchanged; its only diff is a docstring correction, hence a patch bump. The compat floors move to 4.6, the first Core release declaring these `public`, which ExplicitImports requires of an explicit import. Also corrects two docstrings that conflated the two threading families. `QPRK98`'s `thread` field is a FastBroadcast mode passed to `@..`, not an `AbstractThreadingOption`, so telling users to pass `BaseThreads()` there was wrong independently of this change. Closes SciML#4163. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F75XsVeZ94QH3PmCuq6QUF
ChrisRackauckas-Claude
force-pushed
the
threading-option-exports
branch
from
August 9, 2026 14:42
91c7052 to
d5a5891
Compare
threading = PolyesterThreads() resolvesOrdinaryDiffEq.PolyesterThreads() resolves
ChrisRackauckas
marked this pull request as ready for review
August 9, 2026 14:58
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the second half of #4163. The
verbosehalf shipped in #4189; this is the threading half.What changed and why
Sequential,BaseThreadsandPolyesterThreads— the values for thethreadingkeyword — are public API of OrdinaryDiffEqCore, but in v7 nothing downstream carried them, soOrdinaryDiffEq.PolyesterThreads()stopped resolving. OrdinaryDiffEq and the four sublibraries whose solvers takethreading(Extrapolation, FIRK, PDIRK, PRK) now import the names and declare thempublic.The sublibraries matter because the umbrella exports only six algorithms: an extrapolation user reaches the solver through
using OrdinaryDiffEqExtrapolation, so that package has to carry the names itself. Today exactly one of the fifteen module/name combinations resolves by accident —OrdinaryDiffEqExtrapolation.PolyesterThreads, because that module happens to import it for internal use.public, notexport— and why the export route was rejectedAn earlier revision of this PR exported the names. That was the wrong shape, and the evidence is that bare
PolyesterThreads()never worked in v6:threading = OrdinaryDiffEq.PolyesterThreads(), qualified.using OrdinaryDiffEqCore: …, PolyesterThreads— an import, not an export.So the thing v7 actually broke is the qualified spelling.
publicrestores exactly that and nothing more, rather than inventing new surface and pushing a name as generic asSequentialinto every user's namespace. Bare use remains anUndefVarError, as it was in v6, and there is a test asserting that it stays one.A consequence worth noting: since Core already declares these
public, OrdinaryDiffEqCore's source is unchanged by this PR. Its only diff is a docstring correction, so it takes a patch bump rather than a minor one.Two docs bugs fixed along the way
There are two unrelated threading families and the docs conflated them:
threadingtakesSequential()/BaseThreads()/PolyesterThreads()(OrdinaryDiffEqCore) — Extrapolation, FIRK, PDIRK, PRK.threadtakesFastBroadcast.Serial()/Threaded()— passed straight to@.. thread = thread.QPRK98's docstring told users to passBaseThreads()/PolyesterThreads()tothread, which is wrong regardless of this PR.AbstractThreadingOption's docstring also saidthread =. Both corrected.Versions and compat
Compat floors on OrdinaryDiffEqCore raised to 4.6:
4.44.644.644.64.124.6.0 is the first Core release declaring all three
public, which ExplicitImports'all_explicit_imports_are_publiccheck requires. Verified two ways: reading thepublicblock of all 15 installed Core 4.x releases (4.0.0–4.5.0 have none, 4.6.0–4.13.0 have all three), andgit log -Slocating the commit that added it (661f605637, while the tree still read 4.5.0). Not raised to 4.15: nothing here needs a Core newer than 4.6, and the registry tops out at 4.13.0.Two things the test runs caught
publiccounts as a reexport to SciMLTesting.reexports_allowis still needed even though nothing is exported. Removing it fails root QA:names()includespublicnames on Julia ≥ 1.11. An earlier draft of the test asserted!(name in names(mod))to mean "not exported"; that is wrong and failed 15/30.Base.isexportedis the correct check:Verification
Failing before / passing after,
test/InterfaceI/threading_option_public_tests.jlagainst unfixed master:(The 2 passes are the pre-existing accidental
OrdinaryDiffEqExtrapolation.PolyesterThreads.) With the change:Rebase note
This branch was rebased after #4184 (
Default threaded implicit extrapolation to GenericLUFactorization) merged; that PR touches the same Extrapolation module file and the code path the new test exercises, so the test and Extrapolation QA were re-run against the rebased tree rather than trusting a conflict-free rebase.Unrelated observation for whoever cuts the next release: #4184 shipped a behaviour change to OrdinaryDiffEqExtrapolation without bumping its version (master was still 2.4.2). The 2.5.0 bump here will carry it.
Not verified
GROUP=QA(root + each touched sublibrary) andGROUP=InterfaceIonly — not the full suite, and not themultithreadinggroup.Please ignore until reviewed by @ChrisRackauckas.