fix(cuda): drain external vertex force device buffer when cleared - #463
Conversation
There was a problem hiding this comment.
Code Review
This pull request fixes a bug where stale external forces persisted in the device buffer after being cleared on the host. The fix ensures that the device-side force and vertex ID buffers are always resized to match the host vectors, even when empty, and adds a regression test to verify this behavior. The reviewer suggested a cleaner and safer approach to copy the data by using the container-based copy_from API instead of passing raw pointers via .view().copy_from(...).
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
90aae03 to
5bd934b
Compare
When users clear FEM external forces (host h_forces becomes empty), the old code's early-return-on-empty branch skipped the device-side sync entirely. The device-side 'forces' / 'vertex_ids' buffers retained the previous frame's contents and kept getting scatter-added every advance() call — phantom forces that the user thought they had cleared. Fix: always resize() the device buffers to match the host size (zero when cleared). The conditional copy_from() now only runs when there's actual data to copy. Switch to .view().copy_from(...data()) form for correct buffer view semantics.
Asserts that after clearing all external forces (host arrays empty), the FEM vertex's velocity DECREASES on the next advance (elastic restoring). With the buggy code path the device-side force buffer retains the previous frame's contents and the scatter-add kernel re-applies the +x force, growing the velocity instead. Test was verified to FAIL on the pre-fix codebase and PASS with the preceding commit applied.
5bd934b to
d217515
Compare
|
The Windows CI failure is unrelated to this PR — |
|
I'm trying to figure out the reason |
The windows-2025 runner image was updated with MSVC 14.51 (VS 2026), which causes nvcc cudafe++ ACCESS_VIOLATION crashes and fmt warning promotions. Pin to windows-2022 (MSVC 14.44) until CUDA toolkit supports the newer compiler. Co-authored-by: Cursor <cursoragent@cursor.com>
CI Failure AnalysisThe xmake CI failure on Root CauseBetween PR #460 (merged May 19, passed CI) and now, GitHub updated the
Both errors also appear on FixI've pushed a fix in branch Could you please merge |
…/libuipc into pr/fix-fem-force-drain
Summary
FiniteElementExternalVertexForceConstraint::Impl::step()builds host arraysh_forces/h_vertex_idsby filtering the per-vertexis_constrained/external_forceattributes each frame, then copies them to the device buffers. The copy is guarded byif (!h_forces.empty()), so when the user clears all external forces (host arrays empty), the device-side buffers retain the previous frame's contents.Downstream,
FiniteElementExternalVertexForce::do_stepreadsforces.size()from the device buffer; since the device size is never updated to 0, the scatter-add kernel re-applies the stale forces everyadvance()— forever.Fix
Always
resize()the device buffers to match the host array sizes (zero when cleared). Conditionalcopy_from()only fires when there is actual data to copy.Repro / regression test
python/tests/sim_case/test_finite_element_external_force_clear.py:vertex_0.vx ≈ +9.99 m/s✓is_constrained[:] = 0), advance.vx_frame2 < vx_frame1(elastic restoring decelerates the vertex).Verified locally:
mainHEAD764d4cf2):vx_frame2 = 14.44 m/s— stale force re-applied (+44.6% growth)vx_frame2 = 7.69 m/s— elastic restoring only (−23%)Test plan
test_finite_element_external_force_clearis picked up and runs in the cuda-only test selection