make_constraint: traverse dof tree when constructing contact constraints and is sparse#929
Conversation
|
I think even with dense jacobians, the tree traversal strategy could be faster? What do you think? We need to zero the memory though, but I'm sure we can find a good way to do it. |
|
@adenzler-nvidia yes, i think the tree traversal could be faster with dense. added a todo for making the dense version performant with tree traversal |
|
@thowell usually I'm in favor of incremental changes and TODOs, but in this case we're adding some complexity (cache kernel, wp.static etc) that we might remove if it turns out that dof tree traversal makes sense for both dense and sparse. Would you mind having a go at seeing whether it helps in both cases and if so we can simplify the changes in this PR? In general I'm not a huge fan of |
|
one reason why it might not make sense to perform dof traversal for dense is that the |
660e2c2 to
b857db9
Compare
|
refactored the code so that pyramidal and elliptic constraints each have 1 path that utilizes dof traversal
this pr main bb81495 |
b857db9 to
79575bf
Compare
erikfrey
left a comment
There was a problem hiding this comment.
Looks good, just one nit
this pr is part of the effort to implement sparse Jacobians #88
when
is_sparse==True, instead of iterating over all dofs to construction contact constraints, traverse dof tree for each contact bodiesmujoco reference: https://github.com/google-deepmind/mujoco/blob/08b4b4144d70c69206f96cf329d5044ae686a1e6/src/engine/engine_core_util.c#L55
humanoid
performance for dense should be unchanged
this pr:
main (bb81495):
performance for sparse should be improved
this pr:
main (bb81495):
notes:
efc.Jis represented in a sparse format and it is not necessary to zero memory on each call tomake_constraintcontact_pyramidalandcontact_ellipticwith awp.funcintroduced overhead? as a result, to maintain performance for now there is duplicated code in dense and sparse cases.todo