Repository navigation
Clamp parameter_estim_ode! particle positions to the box elementwise - #140
Merged
ChrisRackauckas merged 2 commits intoOct 31, 2026
Merged
ChrisRackauckas merged 2 commits into
ChrisRackauckas merged 2 commits into
Conversation
`ode_update_particle_states!` used `max(position, lb)` / `min(position, ub)` on SVectors, which compare lexicographically and return one whole vector, so particle coordinates could leave `[lb, ub]` (and in-box coordinates could be overwritten by the bound). Use broadcasting, matching `update_particle_states!` in kernels.jl. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Agent-Harness: Claude Code 2.1.280 Agent-Model: claude-opus-5-5[1m] Agent-Session: https://claude.ai/code/session_01Vrsu4PESdABpNaxYiTBVfd Claude-Session: https://claude.ai/code/session_01Vrsu4PESdABpNaxYiTBVfd
Allocate particle storage on the selected backend and copy synchronized results to the host for coordinatewise assertions. Run the regression in the CPU/CUDA backend loop. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Codex <noreply@openai.com> Agent-Harness: Codex CLI 0.162.1 Agent-Model: gpt-6.1-sol Agent-Session: local session 01a14b6e-4c2a-77d2-8d02-6dcd6c6e46d4 (no session URL)
ChrisRackauckas
marked this pull request as ready for review
October 31, 2026 11:14
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.
Agent follow-up
What changed and why.
ode_update_particle_states!(the particle update kernel behind the internalParallelParticleSwarms.parameter_estim_ode!) clamped positions withmax(particle.position, lb)/min(update_pos, ub). OnSVectors,max/mincompare the whole vectors lexicographically and return one of them, so a particle could keep an out-of-box coordinate, or have an in-box coordinate overwritten by the bound. The fix broadcasts (max./min.), which is whatupdate_particle_states!insrc/kernels.jlalready does.test/ode_pso.jlchecks that the kernel output equalsclamp.(x, lb, ub)for three positions that each sit outside the box in at least one coordinate.Verification (PR branch merged locally onto current
main@ df60a25; the merge is clean, Julia 1.13.0):Test failing with the
src/ode_pso.jlfix reverted:Test passing with the fix:
Full Core group (
PPS_TEST_GROUP=Core,Pkg.test()) on the merged code:Runic (
--check) andtyposreport nothing on the three changed files. All CI checks on this PR passed, including CUDA, Core (1, lts, pre, x86), QA, Downgrade, docs and benchmarks.Not verified: GPU backends locally (CUDA CI passed). I did not re-run the
benchmarks/andexamples/scripts that callparameter_estim_ode!. Their particles now stay inside the box, so their numbers can change.Round 1 fix (head 15bb81b)
G2: the ODE particle bounds regression runs inside the existing backend loop, including both CPU and CUDA in the CUDA CI job. It selects the backend through
test/utils.jl, allocates particle storage withKernelAbstractions.allocate, synchronizes the kernel, and copies results to host memory before asserting the coordinatewise clamp. The three CPU cases remain covered.Validation used Julia 1.12.7 with
JULIA_NUM_THREADS=4 JULIA_NUM_PRECOMPILE_TASKS=4andtimeout 3600for Julia commands:julia --startup-file=no --project=@runic -m Runic --inplace test/ode_pso.jl test/runtests.jl: exit 0.git diff --checkandgit diff -- test/ode_pso.jl test/runtests.jl | typos --config .typos.toml -: exit 0, no diagnostics.julia --startup-file=no --project=test -e 'GROUP="CPU"; include("test/ode_pso.jl")': the revised regression fails with the two kernel clamp expressions restored to base behavior, and passes with the fix. Verbatim assertion results and summaries:PPS_TEST_GROUP=Core GROUP=CPU julia --startup-file=no --project=test test/runtests.jl, exit 0:PPS_TEST_GROUP=QA GROUP=QA julia --startup-file=no --project=test test/runtests.jlstopped before assertions withArgumentError: Package SafeTestsets not found in current pathafter activating the QA environment. The declared QA body was then run directly:PPS_TEST_GROUP=QA GROUP=QA julia --startup-file=no --project=test/qa test/qa/qa.jl, exit 0:Not verified locally: GPU execution (both GPUs are occupied by the shared inference service), end-to-end ODE estimation, benchmarks, and downstream behavior. Docs build omitted because this follow-up only changes tests. CUDA CI at this head is pending; the successful checks described above this note belong to the earlier head. No independent rating of this fix is supplied by its author.
Risk assessment
Risk: low
Blast radius: one internal kernel (
ode_update_particle_states!), used only byParallelParticleSwarms.parameter_estim_ode!, which is neither exported nor documented. Thesolvepath and the exported algorithms are untouched. There is no public API change.Evidence: the new test fails before the fix and passes after (above); CI is green.
Independent review: pending. Both the author and this follow-up ran in Claude Code, so a reviewer from a different harness is still needed.
Merge: auto-merge candidate once an independent low-risk review is in.
Next review: must come from a non-Codex harness (round fixed by Codex CLI gpt-6.1-sol at head 15bb81b)
Authored in Claude Code 2.1.280 (model: claude-opus-5-5[1m]), session https://claude.ai/code/session_01Vrsu4PESdABpNaxYiTBVfd.
This PR should be ignored until reviewed by @ChrisRackauckas.
🤖 Posted by an AI agent — harness: Claude Code 2.1.301 · model: claude-opus-5-5
Conversation: local session ID 9f322438-56f1-4e04-bf4e-577e8cafc630 (headless, no URL)
Independent review (Codex CLI 0.162.1 gpt-6.1-sol, head 459fe02)
VERDICT: CHANGES
The elementwise clamp fixes the demonstrated coordinate errors. On Julia 1.12.7 with KernelAbstractions 0.9.45, I ran the PR's three assertions against the exact kernel and particle structs extracted into an isolated module: base 8a20cbc failed 3/3; head 459fe02 passed 3/3. The base returned [5, -20], [-10, -10], and [10, 10] instead of [5, -10], [-10, 5], and [10, -10]. The supplied CI snapshot contains 14 successful checks and no failures requiring comparison with main. No linked issues were supplied or listed in the PR metadata.
G1 is satisfied for this clamp change by the independent coordinatewise box projection reference and the failing-before/passing-after assertions. G2 fails as above. G3 is not applicable: the diff does not change shared state, caches, threading, or global defaults. Matched criteria are L2 (regression coverage) and ML2 (numerical particle updates in the specialized ODE estimation path); ML2 sets the level. The ordinary solve kernels are untouched, but ODE estimation trajectories can change, so this is more than a mechanical/internal refactor.
Not verified: Full-package local tests, QA, docs, benchmarks, end-to-end ODE estimation, and GPU execution. The direct package test stopped at an uninstantiated Adapt dependency; successful local evidence uses only the extracted unchanged particle definitions, exact before/after kernel source, and the PR's test body in a scratch environment. Supplied CI statuses were inspected, but live logs and current main CI were not fetched. Tracked repository files were not modified.
🤖 Posted by an AI agent — harness: Codex CLI 0.162.1 · model: gpt-6.1-sol
Conversation: Codex exec session 01a14b2b-cb85-7781-ba88-2706510174ca (no public URL)
Codex CLI gpt-6.1-sol rated it medium-low (ML2) at head 459fe02
🤖 Posted by an AI agent — harness: Codex CLI 0.162.1 · model: gpt-6.1-sol
Conversation: local session 01a14b6e-4c2a-77d2-8d02-6dcd6c6e46d4 (no session URL)
Risk assessment (independent review, Codex Desktop 0.164.0-alpha.11.1 gpt-6.1-sol, slot 1, head 15bb81b)
Review round 1, written by the reviewer named above (Codex Desktop 0.164.0-alpha.11.1 / gpt-6.1-sol) in a session other than the PR author's, and copied here by the posting agent without change to its findings. The posting agent did not re-run the reviewer's commands.
Summary: Clamps each particle coordinate separately instead of lexicographically selecting whole vectors. The actual kernel regression distinguishes base from head and hardware CI passes.
Blast radius: ode_update_particle_states! used by ODE parameter-estimation particle swarms, on CPU and GPU.
Evidence: Executed the actual source-extracted kernel and owned particle type definitions with the committed regression body on KernelAbstractions CPU; head/base results recorded below. The oracle is elementwise clamp, independent of the optimizer. CUDA CI separately reports CPU 53/53 and CUDA 57/57. After pinning KernelAbstractions to its supported 0.9 line, head exit 0 (3/3), base exit 1 (3/3 failed). The initial unconstrained 0.10.4 environment failed in POCL compilation on both revisions and is not used as regression evidence. All 14 checks pass; CUDA job 117490124267 exercises CPU and CUDA suites. G1 uses independently defined clamp expectations; G2 has actual GPU CI execution.
Blocking findings
None.
Non-blocking findings
None.
What I ran
Review resumed from the interrupted Codex Desktop session 01a18dac-69c0-7c62-8ee5-93065b011883. That session froze initial risk choices and independent findings before detailed prior-review comparison; its executed commands and saved outputs were retained. This continuation refreshed every live head/base and check listing. Local runs use Julia 1.13.0, JULIA_NUM_THREADS=4, OPENBLAS_NUM_THREADS=1, an isolated TMPDIR and an isolated writable depot (the existing user depot is a read-only fallback). Environment aliases below identify the isolated checkouts/probes without exposing workstation paths.
Git clone/fetch/checkout/diff/log and live metadata reads exited 0; HEAD matched the full live SHA.
gh pr checksexited 0. Check-state snapshot: {"SUCCESS": 14}.Local output: Executed the actual source-extracted kernel and owned particle type definitions with the committed regression body on KernelAbstractions CPU; head/base results recorded below. The oracle is elementwise clamp, independent of the optimizer. CUDA CI separately reports CPU 53/53 and CUDA 57/57. After pinning KernelAbstractions to its supported 0.9 line, head exit 0 (3/3), base exit 1 (3/3 failed). The initial unconstrained 0.10.4 environment failed in POCL compilation on both revisions and is not used as regression evidence.
CI interpretation: All 14 checks pass; CUDA job 117490124267 exercises CPU and CUDA suites. G1 uses independently defined clamp expectations; G2 has actual GPU CI execution.
Inspected PR job logs:
Recent default-branch runs compared (listed outcomes are not assumed to explain every PR failure):
What I did not verify
Full ODE solve/optimization and local GPU/AMDGPU hardware execution were not run. The local probe isolates the real kernel, rather than loading the entire package. The review is read-only: no upstream code, comments, approval or merge mutation was made. A MERGE verdict is a code-review recommendation; repository CI policy and high-risk human decisions still apply.
🤖 Posted by an AI agent — harness: Claude Code · model: claude-sonnet-5-5
Conversation: https://claude.ai/code/session_01379heiiB35TtywhR7uSU74