Unit test and parameter checks for scale randomization #1810
Conversation
hapatel-bdai
requested review from
kellyguo11,
jsmith-bdai and
Mayankm96
as code owners
hapatel-bdai
changed the title
source/extensions/omni.isaac.lab/test/envs/test_scale_randomization.py
Outdated
Show resolved
Hide resolved
source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_marl_env.py
Outdated
Show resolved
Hide resolved
| @@ -126,6 +143,10 @@ def randomize_scale( | |||
| # spawn single instance | |||
| prim_spec = Sdf.CreatePrimInLayer(stage.GetRootLayer(), prim_paths[env_id]) | |||
|
|
|||
| # check to make sure xformOp:scale attribute exists | |||
| if not prim_spec.GetAttributeAtPath(prim_paths[env_id] + ".xformOp:scale"): | |||
There was a problem hiding this comment.
similar to comment in #1165, would it be more user friendly to create the scale attribute if one doesn't already exist?
There was a problem hiding this comment.
I was not able to add attributes to Sdf's like we do for Usd's. For example the https://openusd.org/dev/api/class_usd_geom_gprim.html#a7fd86a311b27d9b4fdd82736bb423056 does not exist for type Sdf (prim_spec)
| @@ -63,6 +63,10 @@ def randomize_shape_color(env: ManagerBasedEnv, env_ids: torch.Tensor | None, as | |||
| # spawn single instance | |||
| prim_spec = Sdf.CreatePrimInLayer(stage.GetRootLayer(), prim_paths[env_id]) | |||
|
|
|||
| # check to make sure input:diffuseColor attribute exists | |||
| if not prim_spec.GetAttributeAtPath(prim_paths[env_id] + "/geometry/material/Shader.inputs:diffuseColor"): | |||
There was a problem hiding this comment.
Left a comment in #1165 regarding whether we should create a material if one doesn't exist.
There was a problem hiding this comment.
Agree. We should just use replicator for this randomization. Gets complex to do it on our end.
There was a problem hiding this comment.
Are you suggesting re-doing how the randomization is performed? Can we do this in a follow-up PR? Or do you have suggestions on how this would be done?
There was a problem hiding this comment.
Also, @kellyguo11 any tips on how to create the material if it doesn't exist?
source/extensions/omni.isaac.lab/test/envs/test_scale_randomization.py
Outdated
Show resolved
Hide resolved
source/extensions/omni.isaac.lab/test/envs/test_scale_randomization.py
Outdated
Show resolved
Hide resolved
source/extensions/omni.isaac.lab/test/envs/test_scale_randomization.py
Outdated
Show resolved
Hide resolved
source/extensions/omni.isaac.lab/test/envs/test_scale_randomization.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks a ltof or the MR and the test!
Might make sense to also check that all assets in the scene are created successfuly. Sometimes PhysX might be erroring out which isn't desired.
Also you should add the failure case of replicate physics is not True and sim is playing so we can ensure we handle that.
Description
Added a unit test for the scale randomization feature and added some parameter checks based on comments from #1165
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there