Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PyMJCF Attachment frame extra body breaks compilation of freejoint object attachments #407

Closed
guyazran opened this issue Jul 4, 2023 · 6 comments

Comments

@guyazran
Copy link

guyazran commented Jul 4, 2023

Description:

Attaching one PyMJCF model (the "source") to another (the "destination") adds the content of the source's worldbody into a newly generated body element in the destination worldbody. So if we want to add a robot whose model name is "robot1" to some scene, then all the contents of its worldbody will be inside a body named "robot1/" and all its children's names will be "robot1/{original_name}". The issue is that if "robot1" has a freejoint in its root body (the one direct child of worldbody), the resulting merged model cannot be loaded with MuJoCo because the freejoint element is now inside the base element which is inside the newly generated body "robot1/".

Error Message:

Error: free joint can only be used on top level

Reproduce:

A simple example to recreate the error message using the MuJoCo menagerie. This works with any free-moving robot (has a freejoint in the base). In the "scene.xml" file, we comment the <include/> line to avoid loading the robot this way, and only attach it with MJCF

from dm_control import mjcf

scene_model = mjcf.from_path('mujoco_menagerie/robotis_op3/scene.xml')  # <include/> tag is commented
robot_model = mjcf.from_path('mujoco_menagerie/robotis_op3/op3.xml')

scene_model.attach(robot_model)

physics = mjcf.Physics.from_mjcf_model(scene_model)

Suggested Solution:

My preferred solution would be to add an argument in the attach method that will exclude the outer "wrapper" body. We can still avoid name ambiguity by renaming the source model's components as before ("robot1/base", "robot1/wrist", etc).

@yuvaltassa
Copy link
Collaborator

attach() has several limitations, but this a codebase we are no longer developing and are currently working on a new, long term solution. In the meantime you are welcome to send your proposed solution as a PR, or edit you models locally to work around these limitations (e.g. removing the free joint in the child model and adding it back during attachment).

@guyazran
Copy link
Author

guyazran commented Jul 7, 2023

Thanks for the quick response. Is your long-term solution development watchable on GitHub?

@saran-t
Copy link
Member

saran-t commented Jul 7, 2023

I can expand on @yuvaltassa's answer. This isn't really a limitation of attach per se, but more of a design decision (that you are free to disagree with).

The API was designed under the assumption that the caller adds DoF as required, and that objects should not define its own relationship with the world (in this case, it shouldn't be bringing in its own free joint). The idea being that it allows the user to decide whether the prop should be a free object, a hinged object, a fixed object, or whatever.

@yuvaltassa
Copy link
Collaborator

No, unfortunately.

@guyazran
Copy link
Author

guyazran commented Jul 9, 2023

I've been trying to implement this. My current solution is to edit the _AttachableElement class's attach method such that it extends the frame's siblings list with all the attachment children instead of inserting the frame itself. I want to be able to preserve relative pose according to the attachment site. That is, if i attach an object whose root body position ("pos" attribute in XML) is [1, 0, 0] and the site base position is [0, 1, 0], then final position relative to the attaching frame would be [1, 1, 0]. This can be done manually by editing the properties directly on the root body of the attachment, and is easy for position (simple addition). Are there any helper functions in MuJoCo that can help me do this for rotation?

So far I found some conversion functions such as mju_axisAngle2Quat (in mujoco) and quat2euler/euler2quat (in dm_control). Are there more conversion functions that will help me convert between all 5 types of rotations? Is there an easier way that does not require conversion?

@guyazran
Copy link
Author

Update
I've implemented the required changes. Please see the pull request here. I ended up implementing some required rotation functions as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants