Decouples rigid object and articulation asset classes #644
Conversation
Signed-off-by: Mayank Mittal <[email protected]>
|
|
||
| torch.zeros(1, device=self.sim.device) # dummy tensor to initialize CUDA context | ||
|
|
There was a problem hiding this comment.
| torch.zeros(1, device=self.sim.device) # dummy tensor to initialize CUDA context |
| @@ -116,6 +116,10 @@ def is_fixed_base(self) -> bool: | |||
| """Whether the articulation is a fixed-base or floating-base system.""" | |||
| return self.root_physx_view.shared_metatype.fixed_base | |||
|
|
|||
| @property | |||
| def num_instances(self) -> int: | |||
There was a problem hiding this comment.
Does this need a doc string?
My interpretation of "num_instances" in this case is the number of replications of the articulation. Is this the correct interpretation? Can this be different than the number of environments?
There was a problem hiding this comment.
For most learning setup, "num_instances" = "num_envs". At least that's the main underlying idea. However, for a single environment case, the view can be used to capture two instances of the same robot in that environment.
Scaling it to N environments with M same robots in each remains a discussion point because all the tensor shapes change in that case. We can think of adding it if we see a huge perf gain by doing so, i.e. one view capturing M robots vs. one view per robot.
There was a problem hiding this comment.
That sounds reasonable. What about learning setups for multi-robot collaboration (i.e. two arm manipulation) do we set it up such that the two robots are viewed as one?
| from .articulation_data import ArticulationData | ||
|
|
||
| if TYPE_CHECKING: | ||
| from .articulation_cfg import ArticulationCfg | ||
|
|
||
|
|
||
| class Articulation(RigidObject): | ||
| class Articulation(AssetBase): | ||
| """An articulation asset class. |
There was a problem hiding this comment.
Can you do a Ctrl-F for any instances of rigid + object and remove / update them? The docstring has some references to it being a subclass
|
This seems like a big change, do you mind outlining the pros / cons of this approach? It looks like we're not getting much benefit at first glance, but are now having a lot of repeated code that will be difficult to maintain |
Description
Since we override a lot of the functions from RigidObject inside the Articulation class, we don't need to rely on inheritance anymore. Duplicacy in the code makes it easier to understand the two classes' functionalities without severely added overhead from the maintenance side. Moreover, conceptually, it can be motivated that the two are independent concepts.
This MR decouples the rigid object and articulation concepts in the framework.
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there