Skip to content

Reduce over scalar costs when picking the best particle - #151

Merged
utkarsh530 merged 4 commits into
SciML:mainfrom
AdityaPandeyCN:best-particle-scalar-reduce
Oct 25, 2026
Merged

utkarsh530 merged 4 commits into
SciML:mainfrom
AdityaPandeyCN:best-particle-scalar-reduce

Conversation

@AdityaPandeyCN

@AdityaPandeyCN AdityaPandeyCN commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Problem

minimum(gpu_particles) and minimum(block_particles) reduce over whole SPSOParticle/SPSOGBest structs. On GPU the reduction stages each element in shared memory, so large-dimension positions overflow it and the kernel launches with zero threads.

Fix

Add _best_particle, which runs argmin over the scalar costs and then copies back only the winning element. NaN costs map to typemax, so they never win. Without this, argmin treats NaN as the smallest value, and the old isless-based min let a NaN win depending on its position.

It replaces every minimum reduction in src/:

  • the ParallelSyncPSOKernel block reduction over SPSOGBest (lowerlevel_solve.jl)
  • the CPU sync and async ParallelPSOKernel solvers
  • particle init (init.jl)
  • ODE parameter estimation (ode_pso.jl)

On plain Arrays (the CPU backend), it uses argmin(f, ps) directly, so it allocates nothing.

Tests

  • Added a D = 128 case to the "reduction is dimension-independent" test in test/gpu.jl.
  • The Core tests pass locally on the CPU backend. I couldn't run the GPU path locally (no GPU), so CUDA CI is the real check here.
  • Changed the "block argmin matches brute force" test so it no longer depends on rounding. It compared sol.objective (prob.f recomputed on the host) to the GPU-computed best_cost with ==, which failed by 1 ulp on CUDA, including on main. It now checks the winning position exactly and the cost with ≈.

GPU `minimum` over whole particles/gbests stages each element in shared
memory, so large-dimension positions overflow it and the reduction launches
with zero threads. Pick the best via `argmin` over costs instead, mapping NaN
costs to typemax so they never win. This covers the sync kernel's block
reduction over SPSOGBest as well as the particle reductions in init, the
async/CPU solvers, and ODE parameter estimation.

Adds a 128-dimension case to the dimension-independence GPU test.
sol.objective is prob.f recomputed on the host, so it can differ from the
device-computed best_cost by an ulp on CUDA (also seen on main). Check the
copied position exactly and the cost approximately.
@AdityaPandeyCN

Copy link
Copy Markdown
Contributor Author

@utkarsh530 Please take a look

AdityaPandeyCN added a commit to AdityaPandeyCN/SciMLBenchmarks.jl that referenced this pull request Oct 21, 2026
… fix

Pins ParallelParticleSwarms to 73dee70c from SciML/ParallelParticleSwarms.jl#151.
On the GPU, minimum() over whole SPSOParticle/SPSOGBest structs staged
each element in shared memory, so the 86-dimensional B5 particles
overflowed it and the kernels launched with zero threads, leaving every
GPU swarm run at best = Inf. The PR reduces over the scalar costs instead.

Move the pin to the merged commit or a release once SciML#151 is merged.

Copy link
Copy Markdown
Member

Thanks for this fix. Reducing cost/index candidates instead of complete particle records is the right direction for the shared-memory issue, and the CUDA CI run passed. Reviewing head 73dee70, I have three small correctness/test requests before merging:

  1. Distinguish NaN from a valid +Inf cost (src/ParallelParticleSwarms.jl:81–91). For floating-point costs, typemax(c) == Inf, so costs [NaN, Inf] become [Inf, Inf] and the first particle still wins with its original NaN cost. This is reproducible with the helper on a CPU array. Could we use an explicit validity-aware cost/index comparison, define the all-NaN outcome, and add CPU/CUDA regressions for both SPSOParticle and SPSOGBest? Useful cases are [NaN, Inf], its reverse, mixed finite/NaN, -Inf, and all NaNs.

  2. Include global_update=false in the high-dimensional regression (test/gpu.jl:60–85). Its final selection also changed (lowerlevel_solve.jl:125), but the new D=128 cases only exercise the synchronous and shared-global-best variants. Adding the independent-particle variant, preferably with a ragged population such as 257 particles, would cover that endpoint.

  3. Allow tied minimizers in the exact-position check (test/gpu.jl:103–107). CPU argmin chooses the first minimum, while the block reduction need not: its comparison tree selects particle 3 for costs [9, 1, 1, 9]. The shared-global-best variant can also retain an incumbent on ties. The ≈ cost comparison makes sense, but the position assertion should accept any minimizing particle unless we explicitly implement lowest-index tie-breaking. A deliberate tie test would make this clear.

Two follow-ups:

  • The synchronous block kernel still compares raw costs at src/kernels.jl:143–172. For a block with [NaN, 1.0], it emits the NaN candidate and discards the finite one before _best_particle(block_particles) runs. This predates the PR and can be handled separately, but the new helper does not make the whole synchronous path NaN-safe.
  • Please check performance before treating allocation-free CPU selection as performance-neutral. The LTS benchmark reports synchronous solves increasing from 0.257 to 0.552 ms; the current-Julia result is much closer. This needs reproduction before assigning a cause. On GPU, the cost-array materialization, index readback, and subsequent winning-record readback add work, particularly in the synchronous per-generation path. The existing benchmarks use CPU, so there is no demonstrated GPU slowdown; a warmed, explicitly synchronized GPU comparison would establish the tradeoff.

Validation here was isolated Julia helper tests plus host replay of the block comparisons; I did not run GPU kernels locally.

…zers

- `_best_particle` reduces over (cost, index) with `isless`, so NaN ranks after
  every number including Inf ([NaN, Inf] now picks Inf), ties go to the lowest
  index, and an all-NaN population yields the first particle. The CPU path is
  a plain loop over the same ordering.
- The sync block kernel compares with `isless` and pads with NaN, so a NaN
  particle no longer displaces a finite one within a block.
- Tests: helper regressions for SPSOParticle and SPSOGBest ([NaN, Inf], its
  reverse, mixed finite/NaN, -Inf, all NaN, ties); a NaN-objective solve for
  every kernel variant; global_update = false (including a ragged 257-particle
  population) in the D = 128 test; the brute-force check accepts any tied
  minimizer and adds a tie-heavy objective.
@utkarsh530
utkarsh530 merged commit c147a18 into SciML:main Oct 25, 2026
14 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