-
Notifications
You must be signed in to change notification settings - Fork 932
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
Add APIs for deformable asset #630
Conversation
409ada0
to
3f7669c
Compare
Signed-off-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me now.
source/extensions/omni.isaac.lab/omni/isaac/lab/assets/rigid_object/rigid_object.py
Outdated
Show resolved
Hide resolved
return self.root_physx_view.count | ||
|
||
@property | ||
def num_bodies(self) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this property still make sense for deformables? or more to keep align with the rigid object class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly to keep it similar to rigid bodies but probably doesn't make sense there as well 😅 . Mostly it's there for some backwards compatibility.
@@ -0,0 +1,336 @@ | |||
# Copyright (c) 2022-2024, The Isaac Lab Project Developers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also have tests for the sim and collision APIs in the data class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added init tests to check the tensors are of the right shape
might need another formatting run and looks like the tests are failing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great with some minor doc comments (don't feel the need to add them).
Thanks for fixing a lot of things in the other classes too 🚀
if self._root_physx_view._backend is None: | ||
raise RuntimeError(f"Failed to create articulation at: {self.cfg.prim_path}. Please check PhysX logs.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: Can you add this and the other raised Exceptions to the Raises:
section of docstring. Not critical by any means!
"""Deformable body view for the asset (PhysX). | ||
|
||
Note: | ||
Use this view with caution. It requires handling of tensors in a specific way. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: This is very vague, but I think it's copy / pasted from other assets?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. But I think we can remove this note everywhere. It was only back in the days where we had to handle indexing of tensors ourselves. I think users can now do it as well since it isn't so complicated.
# Think: Should we reset the kinematic targets when resetting the object? | ||
# This is not done in the current implementation. We assume users will reset the kinematic targets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make a ticket for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@masoudmoghani Can you please make a ticket on this? I think in general we shouldn't do any hidden operation but maybe worth keeping in mind as more people adopt deformabels.
Internal helper. | ||
""" | ||
|
||
def _initialize_impl(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: Can add Raises section here for all raised exceptions
# Description This MR adds deformable object API to assets in the core framework. The class creates two physics views: one for the deformable object and the other one for the deformable material. Based on these, the users can set and get different nodal information from the solver, as well as randomize deformable material properties. The MR also adds some basic tests and a script in the tutorial that showcases the deformable object. ## Type of change - New feature (non-breaking change which adds functionality) - This change requires a documentation update ## Screenshots Output from: ```bash ./isaaclab.sh -p source/standalone/tutorials/01_assets/run_deformable_object.py ``` https://github.com/user-attachments/assets/9265f4d4-bebf-41b4-9d73-35c558f47a15 ## Checklist - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there --------- Signed-off-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com> Co-authored-by: Mayank Mittal <mittalma@leggedrobotics.com>
Description
This MR adds deformable object API to assets in the core framework. The class creates two physics views: one for the deformable object and the other one for the deformable material. Based on these, the users can set and get different nodal information from the solver, as well as randomize deformable material properties.
The MR also adds some basic tests and a script in the tutorial that showcases the deformable object.
Type of change
Screenshots
Output from:
defo_cube.mp4
Checklist
pre-commit
checks with./isaaclab.sh --format
config/extension.toml
fileCONTRIBUTORS.md
or my name already exists there