Repository navigation
Support linvel and angvel in contact sensors - #1813
Merged
thowell merged 5 commits intoOct 30, 2026
Merged
Conversation
Contact sensors that request linvel or angvel now compute the relative velocity at the contact point in the velocity stage, matching MuJoCo's copySensorData for geom contacts. Models with flex still raise NotImplementedError for these fields. Signed-off-by: Dundy Pasupuleti <dundysm@gmail.com>
Share the contact frame and position reads with normal/tangent/pos, only read root ids and subtree com for linvel, and reuse com when both bodies share a root. Test pre-fills sensordata with inf and drops default hinge. Signed-off-by: Dundy Pasupuleti <dundysm@gmail.com>
Contributor
Author
|
@thowell when you have a moment, could you take a look? this adds linvel and angvel to contact sensors, computed in the velocity stage to match mujoco c copySensorData, and removes the todo NotImplementedError. i also applied the patterns from #1805 and #1809 here: fewer global reads, sensordata pre-filled with wp.inf in the test, and test_data.fixture. |
Collaborator
|
Thanks @dundysm! Nice work adding 1.
|
…-velocity Signed-off-by: Dundy Pasupuleti <dundysm@gmail.com>
Signed-off-by: Dundy Pasupuleti <dundysm@gmail.com>
Contributor
Author
|
thanks @thowell; merged main (no rebase, to avoid a force-push) and addressed everything in 4b4b0e6:
sensor, io and types tests pass on cpu (248 passed, 4 skipped); ruff is clean. |
thowell
reviewed
Oct 30, 2026
Signed-off-by: Dundy Pasupuleti <dundysm@gmail.com>
thowell
approved these changes
Oct 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
contact sensors that request
linvelorangvelraised NotImplementedError (theTODO(team)in io.py). this computes them in the velocity stage, matching mujoco'scopySensorDatafor geom contacts: the velocity of geom[1] relative to geom[0] at the contact point, fromcvelshifted from each body's subtree com, rotated into the contact frame, with z flipped when the match direction is reversed.sensor_contact_vel_adrand runs_eval_sensor_contactinsensor_vel. contact sensors are excluded fromsensor_vel_adrtests:
test_contact_sensor_velocitychecks against mujoco c across data combos, obj/ref specs including flipped direction, mindist/maxforce/default reduce, and 1 and 2 worlds. full suite passes on cpu (2190 passed).@thowell would appreciate a look when you have time.