Add tests for velocity_limit and velocity_limit_sim #1878
Conversation
jtigue-bdai
requested review from
Mayankm96,
jsmith-bdai and
kellyguo11
as code owners
| elif cfg.effort_limit_sim is not None and cfg.effort_limit is None: | ||
| # TODO: Eventually we want to get rid of 'effort_limit' for implicit actuators. | ||
| # We should do this once all parameters have an "_sim" suffix. | ||
| cfg.effort_limit = cfg.effort_limit_sim |
There was a problem hiding this comment.
Ultimately we don't want to set effort_limit for implicit. We want to only use effort_limit_sim to propagate to physx.
There was a problem hiding this comment.
Yeah but till then I think it's okay to have them both the same values and expect to have the same output when you read them? They should be seen as equivalent IMO
There was a problem hiding this comment.
Yeah I get the reasoning. One issue I ran into was inning the test_valid_config. It operates on the registered instances of the cfgs so when it runs them for the second device it has overwritten them so then it raises the value error.
I guess an easy work around is to make a copy of the cfgs before initializing the asset.
There was a problem hiding this comment.
Yeah. We can do a copy inside the class. I think we might be doing it inside the base constructor already but for implicit since we modify before, it isn't getting called
jtigue-bdai
changed the title
jtigue-bdai
force-pushed
the
fix/actuator-model-again-tests
branch
from
e3ed9c3 to
c7b4cfb
Compare
Description
This PR is to add tests for velocity and effort limit settings for implicit and idealpd actuators.
NOTE this is a PR into a fix branch not main.
Fixes # (issue)
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there