support ball joints #56
Conversation
WalkthroughAlright, listen up, you greenhorns! This pull request ain't just a little tune-up; it’s a full overhaul of how joints are handled in Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant JointHandler
participant Model
Controller->>JointHandler: Call _align_joint_dims(types, ranges, names)
JointHandler->>Model: Compute bounds and part names
Model-->>JointHandler: Return lb, ub, part names
JointHandler-->>Controller: Return results
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (2)
Files skipped from review as they are similar to previous changes (1)
Additional comments not posted (5)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
==========================================
+ Coverage 39.87% 41.29% +1.41%
==========================================
Files 9 9
Lines 499 511 +12
==========================================
+ Hits 199 211 +12
Misses 300 300 ☔ View full report in Codecov by Sentry. |
tests/test_controller.py
Outdated
| from jax import numpy as jp | ||
|
|
||
| types = [0, 1, 2, 3] | ||
| ranges = [[0.0, 0.0], [0.0, 1.0], [-1.0, 1.0], [-1.0, 1.0]] |
There was a problem hiding this comment.
Ball joint has four coordinates, but we only list a single min, max pair. Is there a reason for that? Is there a way to specify a ball joint limited to an asymmetric cone?
There was a problem hiding this comment.
I don't think so. Documentation on ball joint range:
For ball joints, the limit is imposed on the angle of rotation (relative to the reference configuration) regardless of the axis of rotation. Only the second range parameter is used for ball joints; the first range parameter should be set to 0.
stac_mjx/controller.py
Outdated
| @@ -25,11 +25,38 @@ | |||
| from tqdm import tqdm | |||
|
|
|||
| # Root position (3) + quaternion (7) in qpos | |||
| _ROOT_QPOS_LB = -jp.inf * jp.ones(7) | |||
| _ROOT_QPOS_UB = jp.inf * jp.ones(7) | |||
| _ROOT_QPOS_LB = -jp.inf | |||
There was a problem hiding this comment.
Seems like spatial can be +- inf, but angular (quat components should be +- 1.0.)
stac_mjx/controller.py
Outdated
| for type, range, name in zip(types, ranges, names): | ||
| dims = _JOINT_TYPE_DIMS[type] | ||
| # Set inf bounds for freejoint | ||
| if type == 0: |
There was a problem hiding this comment.
Thinking, hmm, I don't understand why we have to handle free joint separately?
There was a problem hiding this comment.
see below ⬇️ is there a better way?
|
Updated the quaternion range for freejoint, and used the enum binding instead (I'd forgotten about that, thanks!). Also scramble the test a bit as you suggested. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- stac_mjx/controller.py (4 hunks)
- tests/test_controller.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/test_controller.py
Additional comments not posted (5)
stac_mjx/controller.py (5)
27-29: Looks good, keep on truckin'!The new way of definin' them bounds for the root joint usin' concatenation is mighty fine. It's clearer than a windshield after a bug splatters on a hot summer day.
31-37: Lookin' slick, like a greased pig at the county fair!This here new dictionary
_MUJOCO_JOINT_TYPE_DIMSis a real beaut. It's gonna make handlin' them joint dimensions smoother than a fresh jar of moonshine.
79-79: Well, butter my biscuit! This here change is slicker than snot on a doorknob.Renamin' that
mj_modelvariable toself._mj_modelis a smart move. It's gonna keep things consistent like a well-oiled diesel engine.
84-86: Shucks, this little addition is as smooth as a fresh jar of peanut butter!Grabbin' them body names from
self._mj_modelis a nice touch. It's as clean as a whistle on a freight train.
88-93: Hot dang, this here code is tighter than a truck stop parking lot on a holiday weekend!Snatchin' them joint names and callin' that fancy
_align_joint_dimsfunction is a real game-changer. It's gonna make handlin' them joint properties smoother than a freshly paved highway.
Resolves issue #55.
Minor modifications to the STAC class's
initto accommodate for this change, and uses mujoco native functions for getting joint and body names rather than using dm_control.Wrote a unit test for the new logic.
Summary by CodeRabbit
New Features
Bug Fixes
Tests