Particles: call Redistribute() once per patch instead of once per level

[lwJi]

Summary

• Remove Redistribute() from PushAndDeposeParticles() where it ran redundantly for every (patch, lev) pair
• Add pc->Redistribute() after the level loop in both test thorns so it runs exactly once per patch per time step
• Output is bit-for-bit identical to pre-change behavior

Test plan

• Build succeeds (./agent_scripts/build.sh)
• Tests pass (./agent_scripts/test.sh)
• Manual verification: test output is bit-for-bit identical to pre-change reference output

Closes #34

[lwJi]

@claude review

[claude]

Claude finished @lwJi's task in 1m 13s —— View job

---

Code Review

• Gather context
• Read changed files
• Analyze changes
• Post review feedback

---

Summary

The change removes Redistribute() from the end of PushAndDeposeParticles() and moves it to the call sites — once per patch, after the level loop. This is a clean and correct optimization for the current usage. A few notes below.

---

Correctness

Single-level case (current tests): Behavior is identical. The tests use assert(ghext->num_patches() == 1) and appear single-level, so bit-for-bit identity is expected and verified.

Multi-level AMR consideration: In the old code, Redistribute() ran after each level, which allowed particles to be reassigned to the correct level before being pushed at the next finer level. In the new code, all levels are pushed before any redistribution occurs. If a particle at level 0 moves into a fine-grid (level 1) region after the level-0 push, the level-1 iterator will not see it (it's still assigned to level 0), so it gets pushed only at coarse resolution for that step.

This is likely acceptable if AMR is not yet exercised, but it's worth documenting the assumption or adding a comment for future multi-level work.

API Contract

Redistribute() was previously an encapsulated implementation detail of PushAndDeposeParticles(). Moving it to the caller creates an implicit contract: callers must call Redistribute() after the level loop. A future caller that omits this call would silently get wrong behavior (particles stuck in wrong MPI ranks/tiles).

Suggestion: add a comment to PushAndDeposeParticles() documenting that the caller is responsible for calling Redistribute() after all levels are processed. Fix this →

Minor Nit

The removed comment // tidy after the full step was slightly misleading anyway — it was actually tidying after each level, not after the full step. The new placement (after all levels) better matches the comment's intent.

---

Overall: The optimization is sound for the current codebase. The performance benefit is real — reducing Redistribute() calls from once per (patch, lev) to once per patch eliminates redundant MPI communication. Approved with the suggestion to document the new caller responsibility.