NVIDIA / warp

A Python framework for high performance GPU simulation and graphics
https://nvidia.github.io/warp/
Other
4.07k stars 226 forks source link

[BUG] Potential bug in warp.sim.collide? #272

Open xuan-li opened 1 month ago

xuan-li commented 1 month ago

Bug Description

In warp/sim/collide.py, wp.clone are used to create new arrays when requires_grad=True:

model.soft_contact_body_pos = wp.clone(model.soft_contact_body_pos)
model.soft_contact_body_vel = wp.clone(model.soft_contact_body_vel)
model.soft_contact_normal = wp.clone(model.soft_contact_normal)

...

model.rigid_contact_point0 = wp.clone(model.rigid_contact_point0)
model.rigid_contact_point1 = wp.clone(model.rigid_contact_point1)
model.rigid_contact_offset0 = wp.clone(model.rigid_contact_offset0)
model.rigid_contact_offset1 = wp.clone(model.rigid_contact_offset1)
model.rigid_contact_normal = wp.clone(model.rigid_contact_normal)
model.rigid_contact_thickness = wp.clone(model.rigid_contact_thickness)

It looks like that in the backpropagate, there will be a gradient flow between the new copy and the old one. But the two copies should be independent. I think wp.empty_like should be used here.

System Information

No response

eric-heiden commented 1 month ago

Hi @xuan-li,

Thanks for reporting this bug! You are right, the wp.copy operation now also copies the gradient arrays during the backward pass, so we should use wp.empty_like here. We will fix this for the next release.