Skip to content

Declare the threading options public so OrdinaryDiffEq.PolyesterThreads() resolves - #4196

Merged
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:threading-option-exports
Aug 9, 2026
Merged

ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:threading-option-exports

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Aug 9, 2026 •

Copy link
Copy Markdown
Member

Closes the second half of #4163. The verbose half shipped in #4189; this is the threading half.

What changed and why

Sequential, BaseThreads and PolyesterThreads — the values for the threading keyword — are public API of OrdinaryDiffEqCore, but in v7 nothing downstream carried them, so OrdinaryDiffEq.PolyesterThreads() stopped resolving. OrdinaryDiffEq and the four sublibraries whose solvers take threading (Extrapolation, FIRK, PDIRK, PRK) now import the names and declare them public.

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, not export — and why the export route was rejected

An earlier revision of this PR exported the names. That was the wrong shape, and the evidence is that bare PolyesterThreads() never worked in v6:

  • v6.80's own test suite writes threading = OrdinaryDiffEq.PolyesterThreads(), qualified.
  • The v6.97 umbrella only ever did using OrdinaryDiffEqCore: …, PolyesterThreads — an import, not an export.
  • No OrdinaryDiffEqCore release has ever exported them.

So the thing v7 actually broke is the qualified spelling. public restores exactly that and nothing more, rather than inventing new surface and pushing a name as generic as Sequential into every user's namespace. Bare use remains an UndefVarError, 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:

  • threading takes Sequential() / BaseThreads() / PolyesterThreads() (OrdinaryDiffEqCore) — Extrapolation, FIRK, PDIRK, PRK.
  • thread takes FastBroadcast.Serial() / Threaded() — passed straight to @.. thread = thread.

QPRK98's docstring told users to pass BaseThreads()/PolyesterThreads() to thread, which is wrong regardless of this PR. AbstractThreadingOption's docstring also said thread =. Both corrected.

Versions and compat

package version reason
OrdinaryDiffEq 7.4.0 → 7.5.0 new public API
OrdinaryDiffEqCore 4.14.0 → 4.14.1 docstring fix only
OrdinaryDiffEqExtrapolation 2.4.2 → 2.5.0 new public API
OrdinaryDiffEqFIRK 2.6.2 → 2.7.0 new public API
OrdinaryDiffEqPDIRK 2.2.3 → 2.3.0 new public API
OrdinaryDiffEqPRK 2.2.3 → 2.3.0 new public API

Compat floors on OrdinaryDiffEqCore raised to 4.6:

package floor before floor after
Extrapolation 4.4 4.6
PRK 4 4.6
PDIRK 4 4.6
FIRK 4.12 unchanged

4.6.0 is the first Core release declaring all three public, which ExplicitImports' all_explicit_imports_are_public check requires. Verified two ways: reading the public block 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), and git log -S locating 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

public counts as a reexport to SciMLTesting. reexports_allow is still needed even though nothing is exported. Removing it fails root QA:

┌ Info: run_qa: unapproved public reexports
│   pkg = OrdinaryDiffEq
│   reexported = [:BaseThreads, :PolyesterThreads, :Sequential]
No unapproved public reexports: Test Failed

names() includes public names 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.isexported is the correct check:

names(M)         = [:Bar, :Foo, :M]      # :Foo is public-only, still listed
ispublic(M,:Foo) = true   isexported(M,:Foo) = false

Verification

Failing before / passing after, test/InterfaceI/threading_option_public_tests.jl against unfixed master:

Expression: isdefined(mod, name)
Expression: getproperty(mod, name) === getproperty(OrdinaryDiffEqCore, name)
UndefVarError: `Sequential` not defined in `OrdinaryDiffEq`
ERROR: LoadError: Some tests did not pass: 2 passed, 14 failed, 16 errored, 0 broken.

(The 2 passes are the pre-existing accidental OrdinaryDiffEqExtrapolation.PolyesterThreads.) With the change:

threading options resolve qualified from every module taking `threading` |   32     32
threading options are declared public, not exported                     |   30     30
bare use stays an error                                                 |    3      3
the qualified options drive the solvers                                 |    3      3

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

  • GPU and downstream jobs.
  • GROUP=QA (root + each touched sublibrary) and GROUP=InterfaceI only — not the full suite, and not the multithreading group.

Please ignore until reviewed by @ChrisRackauckas.

…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 ChrisRackauckas-Claude changed the title Export the threading options so threading = PolyesterThreads() resolves Declare the threading options public so OrdinaryDiffEq.PolyesterThreads() resolves Aug 9, 2026
@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review August 9, 2026 14:58
@ChrisRackauckas
ChrisRackauckas merged commit 438ea89 into SciML:master Aug 9, 2026
200 of 263 checks passed
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