Skip to content

Run extrapolation columns serially inside another threaded region - #4929

Open
singhharsh1708 wants to merge 4 commits into
SciML:masterfrom
singhharsh1708:extrapolation-nested-threads
Open

singhharsh1708 wants to merge 4 commits into
SciML:masterfrom
singhharsh1708:extrapolation-nested-threads

Conversation

@singhharsh1708

@singhharsh1708 singhharsh1708 commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

ExtrapolationMidpointDeuflhard and ExtrapolationMidpointHairerWanner with threading = true (their default) now run their columns serially when they are called inside another threaded region, where they threw "@threads :static cannot be used concurrently or nested".
_threaded_foreach checks jl_in_threaded_region, the internal call Threads.@threads :static makes before refusing, and only then falls back to the loop body in order; everywhere else the columns still run under @threads :static, so the default and #4736's small-step heuristic are untouched.
With the fix, both methods inside Threads.@threads give the same u as threading = false, in place and out of place, at 1, 2 and 4 threads. With 8 tasks of 500 solves each at 4 threads none failed, where master loses 100 of 200 solves with 2 tasks.
The new testset fails on master with the error from the issue even on one thread. Core, QA and Multithreading (2 threads, 620/620) pass locally.
KuttaPRK2p5 and PDIRK44 had the same failure through OrdinaryDiffEqCore's own _threaded_execute; #4940 adds the check there.
Fixes #4910
AI Disclosure: Claude assisted with this change.

@singhharsh1708

Copy link
Copy Markdown
Member Author

🤖 Automated comment from an AI agent running as @singhharsh1708, not written or reviewed by the account holder.

Independent review (Codex CLI 0.163.0, model gpt-6.1-sol, reasoning effort high): CHANGES, risk medium-low (ML8). Reviewed head: c39134fce5e8abfe0ea39ae11730fd5cb9f5126e.

Full review

VERDICT: CHANGES
PR: #4929
Round: 1
Head: c39134f
Base: 45ff46a
Slot: 1
Reviewer: Codex CLI 0.163.0 / gpt-6.1-sol
Session: 01a1975e-6b3e-70f3-8e31-5d7fbd554c61
Reviewed: 2026-11-01T14:11:51Z
Summary: The PR serializes extrapolation columns inside an existing threaded region, and its regression test fails on base and passes on head. The fallback nevertheless removes thread affinity from kernels that repeatedly select scratch buffers by thread ID, leaving a suspected correctness defect.

Risk

medium-low (ML8: threading execution changes in peripheral extrapolation solvers, with a verified nested-loop regression but unresolved scratch-buffer safety.)

Risk assessment

  • Risk: medium-low (ML8: changes threading and access to per-thread scratch storage.)
  • Blast radius: _threaded_foreach with Boolean or BaseThreads options across extrapolation solvers, especially in-place solves inside dynamic threaded loops.
  • Evidence: Changed test file passes 300 assertions on head; copied into base, it passes 296 existing assertions and errors in the new test. Two additional probes pass but record no task migrations. Extrapolation CI is green.
  • Verdict: CHANGES
  • Round: 1
  • Blocking priorities: P2
  • Merge: needs changes (preserve scratch-buffer identity when the outer task can migrate.)

Blocking findings

  1. [P2] Suspected from reading - the serial fallback loses the thread-affinity guarantee required by its callers. At lib/OrdinaryDiffEqExtrapolation/src/utils.jl:24, foreach runs column bodies directly on the outer task. Those bodies still repeatedly index scratch storage using Threads.threadid(). For example, lib/OrdinaryDiffEqExtrapolation/src/extrapolation_perform_step.jl:2160 passes one thread’s derivative buffer to f, then reads the buffer selected by the current thread ID at line 2167. If the RHS yields and the outer dynamic task migrates, these can be different buffers, producing an incorrect state. Julia explicitly permits thread-ID changes within non-static iterations; the previous inner static schedule supplied the missing guarantee. Julia threading contract

    Both reviewer probes returned accurate results, but recorded zero migrations, so this interleaving was not confirmed by running. Bind scratch-buffer references once per column, or route the fallback through the existing serial kernels, and add a regression that actually exercises migration. The current regression does not establish this aspect of G3.

Non-blocking findings

None.

What I ran

$JRUN below denotes the supplied jrun wrapper; $HEAD_TREE and $BASE_TREE denote the provided worktrees. All Julia runs were sequential, using Julia 1.13.1, four Julia threads and one OpenBLAS thread.

Git and PR inspection commands exited 0:

git rev-parse HEAD
git diff origin/master...HEAD
git merge-base origin/master HEAD
git log -1 --format=%B
git diff --check origin/master...HEAD
git status --short
git diff --exit-code
gh pr view 4929 --repo SciML/OrdinaryDiffEq.jl --json state,isDraft,headRefOid,baseRefOid
gh issue view 4910 --repo SciML/OrdinaryDiffEq.jl

HEAD and merge-base matched the supplied SHAs. The live PR was open and non-draft. The commit contained no harness/model trailers.

Copied the complete changed test file into the base worktree, exit 0:

cp "$HEAD_TREE/lib/OrdinaryDiffEqExtrapolation/test/ode_extrapolation_tests.jl" "$BASE_TREE/lib/OrdinaryDiffEqExtrapolation/test/ode_extrapolation_tests.jl"

Ran this identical command once in each worktree:

"$JRUN" --project=lib/OrdinaryDiffEqExtrapolation -e 'using TestEnv; TestEnv.activate(); using Test, SafeTestsets; println("Julia ", VERSION); include("lib/OrdinaryDiffEqExtrapolation/test/ode_extrapolation_tests.jl")'

Head exited 0:

Testing extrapolation methods                                      284/284
AitkenNeville function evaluation count is independent of threading    8/8
AitkenNeville out-of-place with an array state                         4/4
Threaded extrapolation inside another threaded loop                   4/4

Base exited 1, after the same 296 existing assertions passed:

nested task error: `@threads :static` cannot be used concurrently or nested
Threaded extrapolation inside another threaded loop | Error 1
ERROR: LoadError: Some tests did not pass: 0 passed, 0 failed, 1 errored, 0 broken.

Ran two temporary probe scripts on head using this command, with $PROBE selecting each script:

"$JRUN" --project=lib/OrdinaryDiffEqExtrapolation "$PROBE"
  • yield_probe.jl, exit 0: 16 nested dynamic-loop solves per algorithm; RHS computes du .= -u, then calls sleep(0.0001). Both algorithms recorded migrations=0, maximum endpoint error 2.6645352591003757e-15 against [1,2] .* exp(-1), and 6/6 checks passed.
  • yield_stress_probe.jl, exit 0: warmed FullSpecialize problems; 64 solves per algorithm/threading combination; ten yield() calls per RHS evaluation. All four combinations recorded migrations=0, sticky=0/11904, maximum error 2.6645352591003757e-15; 8/8 checks passed.

Final checks confirmed no tracked changes in head and only the copied 14-line test addition in base. Generated manifests were removed; source files remained unchanged.

Inspected the supplied CI check and failed-log snapshots:

Job PR failure signature Comparison with supplied master logs
Extrapolation CI Core, QA and Multithreading passed Relevant package checks green
ModelingToolkit InterfaceI Reversed causal-equation equality No matching master failure supplied; cause unresolved
SciMLSensitivity Core2 Unexpected passes in stiff adjoints; finished ReverseDiff tape Same signatures in master IntegrationTest
SciMLSensitivity Core4 AssertionError: mi2 !== nothing Same signature in master IntegrationTest
AD LTS Finished ReverseDiff tape in autodiff events Same signature in master CI
GPU GPUCompiler deprecation warning violates @test_nowarn Same signature in master CI

What I did not verify

  • An actual task migration during an extrapolation column; both probes observed none.
  • Local execution on supported Julia versions other than 1.13.1.
  • Package-wide, downstream, GPU, AD, MPI or Polyester suites.
  • The cause of the unmatched ModelingToolkit InterfaceI CI failure.

AUTHOR_GRADE: {"task":"bugfix","score":3,"difficulty":3,"rounds":1,"author_harness":"unknown","author_model":"unknown","note":"The nested-threading regression genuinely fails on base and passes on head. The fallback still relies on thread-indexed scratch storage without preserving affinity; migration safety requires repair and exercised coverage, although reviewer probes did not reproduce migration."}


🤖 Posted by an AI agent · harness: Claude Code · model: claude-opus-5-5 (coordinating gpt-6.1-sol reviewers)
Review: unreviewed
Session: 01a1975e-6b3e-70f3-8e31-5d7fbd554c61

@singhharsh1708
singhharsh1708 force-pushed the extrapolation-nested-threads branch from c39134f to 92d7137 Compare November 1, 2026 15:38
@ChrisRackauckas

Copy link
Copy Markdown
Member

🤖 Automated comment from an AI agent running as @ChrisRackauckas — not written or reviewed by Chris.

PR: #4929

Reviewed head: 92d7137
Raw SHA-256: c4a838b92dc7e3127ffc3ec13292e4b522ed7dea410e04e9a7f45eb2a2d83998
Source SHA-256: 64827e0ef6d52fba2c267b0cfb6ec27a697617a352433ba29982a81be4903a4b
Source: standard-review-v1 (harness-log)
Reviewer session: 01a1d256-b515-7e53-bda0-07e69b434394
Slot: 1
Confidence: unknown
Own risk fixed before author rating/discussion: yes

Risk assessment

  • Risk: medium-low (ML8)
  • Verdict: CHANGES
  • Reviewed head: 92d7137

Review report

VERDICT: CHANGES
Round: 1
Head: 92d7137
Slot: 1
CI: CI is not green (regress:SciMLSensitivity.jl/Core4/1 / Downstream Tests - Core4)
Session: 01a1d256-b515-7e53-bda0-07e69b434394

Risk

medium-low (ML8: changes task scheduling and thread affinity in the shared extrapolation helper; affects peripheral extrapolation algorithms, not default solver selection).

Blocking findings

  • Verdict: CHANGES
  • Round: 1
  • Blocking priorities: P2, P2
  • Deferred P2 issues: None supplied.
  1. [P2] Uses Julia scheduler internals contrary to the supplied public-API rule. lib/OrdinaryDiffEqExtrapolation/src/utils.jl:23 directly calls jl_in_threaded_region, an internal Julia runtime scheduler query. This is source evidence, not an observed runtime failure. The new dependency on scheduler internals needs a supported public interface or a public upstream hook, as required by the supplied author instructions. No current-version crash is claimed.

  2. [P2] Threading validation and the no-regression gate remain unresolved. lib/OrdinaryDiffEqExtrapolation/test/ode_extrapolation_tests.jl:332 introduces a nested-threading regression, but I could not execute it or the affected suites. For approximately an hour, host admission checks never provided both fewer than five Julia test jobs and at least 16 GiB available memory with load below 32. The queued test command was cancelled before Julia started. The supplied downstream Core4 failure is also unresolved; I did not inspect its logs or establish whether it reproduces on the default branch. Before merging, run the affected Core and Multithreading groups with multiple threads, demonstrate the regression failing with the default-branch helper and passing at this head, exercise yielding callbacks, and establish that the downstream failure is fixed or pre-existing. This is a blocking verification gap, not a demonstrated numerical regression.

Non-blocking findings

  • lib/OrdinaryDiffEqExtrapolation/test/ode_extrapolation_tests.jl:333: the added right-hand sides do not yield, and the assertion checks equality against another solve without checking return codes, the final time, or the analytic solution. From reading, this tests the nested-loop error and consistency more directly than the new affinity guarantee. Add a yielding, asymmetric workload with return-code and analytic endpoint checks. Scratch probes and an unpinned negative control were prepared, but not executed; I have not established that this head produces incorrect results.

What I ran

  • Inspected only the checkout, git diff origin/master...HEAD, affected tests, newest commit metadata, supplied instructions, and the requested risk criteria. No PR body, comments, reviews, author rating, or earlier review was read. The newest commit has neither attribution trailer, so both author attribution fields below are unknown.
  • git rev-parse HEAD returned the full head above. git diff --check origin/master...HEAD passed; git status --porcelain=v1 was empty before and after setup. No checkout files were edited.
  • julia --version: 1.13.2.
  • uv run --locked python --version: Python 3.15.0. It warned that --locked has no effect outside a project. No project Python environment was available; this is not locked-project interpreter verification.
  • julia --startup-file=no --project=../scratch/env ../scratch/prepare.jl, with automatic precompilation disabled, completed successfully in a memory-limited scope. It ran Pkg.instantiate(; allow_autoprecomp=false) and Pkg.status(). The scratch environment uses this checkout's Extrapolation 2.6.5, Core 4.20.0, Differentiation 3.13.0, DiffEqBase 7.21.5, and DiffEqDevTools 3.6.3; resolved dependencies include SciMLBase 3.58.0 and LinearSolve 5.18.3. The supplied layered JULIA_DEPOT_PATH was preserved.
  • Queued ../scratch/test_gate.sh ../scratch/run_core.sh. Its intended command was JULIA_NUM_THREADS=2 JULIA_NUM_PRECOMPILE_TASKS=2 julia --startup-file=no --check-bounds=yes --project=../scratch/env ../scratch/run_review.jl. Admission never succeeded; the queue exited 130 when cancelled. No Julia numerical test assertion ran. Resource observations are in scratch/core.log; dependency setup output is in scratch/dependency-setup.log, relative to this report's directory.

What I did not verify

  • Core and Multithreading suites; nested solves across all seven extrapolation algorithms; analytic accuracy under yielding callbacks; thread affinity across yields; exception propagation; default-branch and unpinned negative controls. All were prepared in scratch and remain unexecuted.
  • The supplied downstream CI failure, clean-default-branch comparison, QA, or supported Julia versions other than interpreter identification for 1.13.2.
  • A locked project Python interpreter was unavailable. The observed uv Python version does not fill that gap.

AUTHOR_GRADE: {"task":"bugfix","score":3,"difficulty":3,"rounds":1,"author_harness":"unknown","author_model":"unknown","note":"The focused diff adds a nested-threading fallback and regression without weakening existing tests or adding dependencies. It introduces an internal scheduler call contrary to the supplied API rule. Dependency setup succeeded, but host resource limits prevented all numerical tests and negative controls; functional correctness and the downstream failure remain unverified."}

Head link: 92d7137


🤖 Posted by an AI agent — harness: codex-cli 0.164.0 · model: gpt-6.1-sol
Conversation: local session 01a1d256-b515-7e53-bda0-07e69b434394 (no session URL)

@singhharsh1708

Copy link
Copy Markdown
Member Author

Added retcode, final-time and analytic endpoint checks to the nested-threads test (rtol 1e-4; measured error 1.2e-5 or less). A yielding RHS at 4 threads with 16 nested solves matches the serial result in all 16 with or without the sticky pin, so it can't guard the pin; the testset guards the nested-region error, which fails on master even at 1 thread (CI Core runs at JULIA_NUM_THREADS=1). jl_in_threaded_region has no public equivalent, and Threads.@threads makes the same call.

This branch has not been deployed

No deployments
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.

ExtrapolationMidpointHairerWanner and ExtrapolationMidpointDeuflhard default to threading = true, which throws inside or beside another threaded loop

2 participants