Repository navigation
Run extrapolation columns serially inside another threaded region - #4929
singhharsh1708 wants to merge 4 commits into
Conversation
Independent review (Codex CLI 0.163.0, model gpt-6.1-sol, reasoning effort high): CHANGES, risk medium-low (ML8). Reviewed head: Full reviewVERDICT: CHANGES Riskmedium-low (ML8: threading execution changes in peripheral extrapolation solvers, with a verified nested-loop regression but unresolved scratch-buffer safety.) Risk assessment
Blocking findings
Non-blocking findingsNone. What I ran
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.jlHEAD 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: Base exited 1, after the same 296 existing assertions passed: Ran two temporary probe scripts on head using this command, with "$JRUN" --project=lib/OrdinaryDiffEqExtrapolation "$PROBE"
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:
What I did not verify
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) |
c39134f to
92d7137
Compare
PR: #4929 Reviewed head: 92d7137 Risk assessment
Review reportVERDICT: CHANGES Riskmedium-low (ML8: changes task scheduling and thread affinity in the shared extrapolation helper; affects peripheral extrapolation algorithms, not default solver selection). Blocking findings
Non-blocking findings
What I ran
What I did not verify
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 |
|
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. |
ExtrapolationMidpointDeuflhardandExtrapolationMidpointHairerWannerwiththreading = true(their default) now run their columns serially when they are called inside another threaded region, where they threw "@threads :staticcannot be used concurrently or nested"._threaded_foreachchecksjl_in_threaded_region, the internal callThreads.@threads :staticmakes 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.@threadsgive the sameuasthreading = 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.
KuttaPRK2p5andPDIRK44had the same failure through OrdinaryDiffEqCore's own_threaded_execute; #4940 adds the check there.Fixes #4910
AI Disclosure: Claude assisted with this change.