-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Added new curriculum mdp that allows modification on any environment parameters #2777
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
base: main
Are you sure you want to change the base?
Conversation
@jtigue-bdai Feel free to view and provide some feedback |
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.
Thanks for this @ooctipus, we don't currently have tests for mdp terms but do you think you could put together a unit test for this? Because it has the potential for touching so many things I think it would be good to get some unit tests for it.
Reads `cfg.params["address"]`, replaces only the first occurrence of "s." | ||
with "_manager.cfg.", and then behaves identically to ModifyEnvParam. | ||
|
||
for example: command_manager.cfg.object_pose.ranges.xpos -> commands.object_pose.ranges.xpos |
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.
In the example here can you show an example use of this so the syntax is clear?
This term compiles getter/setter accessors for a target attribute (specified by | ||
`cfg.params["address"]`) the first time it is called, then on each invocation | ||
reads the current value, applies a user-provided `modify_fn`, and writes back | ||
the result. |
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 add an example code snippet on how you would use this?
if isinstance(self.container, tuple): | ||
getter = lambda: self.container[self.last] | ||
|
||
def setter(val): | ||
tuple_list = list(self.container) | ||
tuple_list[self.last] = val | ||
self.container = tuple(tuple_list) | ||
|
||
elif isinstance(self.container, dict): | ||
getter = lambda: self.container[self.last] | ||
|
||
def setter(val): | ||
self.container[self.last] = val | ||
|
||
elif isinstance(self.container, object): | ||
getter = lambda: getattr(self.container, self.last) | ||
|
||
def setter(val): | ||
setattr(self.container, self.last, val) |
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.
do we need to add a condition for single values (i.e. int, float, bool, etc) or does the object condition handle 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.
we don't need for single values, because the object condition handle this, the type check is checking the container not the last.
for example, observations.policy.joint_pos.unoise.n_min
has value -0.1,
then the self.container
becomes unoise, a object, self.last
becomes n_min.
there are three kinds of container as far as I can think of, tuple, dict, or object. So the condition should be complete
e28803f
to
8957e93
Compare
…hing, and wrote test for this modify_env_param and modify_term_cfg
8957e93
to
60a0b87
Compare
Description
This PR created two curriculum mdp that can change any parameter in env instance.
namely
modify_term_cfg
andmodify_env_param
.modify_env_param
is a more general version that can override any value belongs to env, but requires user to know the full path to the value.modify_term_cfg
only work with manager_term, but is a more user friendly version that simplify path specification, for example, instead of write "observation_manager.cfg.policy.joint_pos.noise", you instead write "observations.policy.joint_pos.noise", consistent with hydra overriding styleBesides path to value is needed, modify_fn, modify_params is also needed for telling the term how to modify.
Demo 1: difficulty-adaptive modification for all python native data type
(float)
(tuple or list)
Demo 3: overriding entire term on env_step counter rather than adaptive
Demo 4: overriding Tensor field within some arbitary class not visible from term_cfg
(you can see that 'address' is not as nice as mdp.modify_term_cfg)
Type of change
Checklist
pre-commit
checks with./isaaclab.sh --format
config/extension.toml
fileCONTRIBUTORS.md
or my name already exists there