Skip to content
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

[bugfix]: remove joint motors when converting objects to MotionType.KINEMATIC #1566

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

aclegg3
Copy link
Contributor

@aclegg3 aclegg3 commented Sep 8, 2023

Motivation and Context

JointMotors created for ArticulatedObjects stick around in the Bullet backend when the AO is removed from the simulation world (set to KINEMATIC). These motors undergo costly constrain row computation every physics step but do nothing. Instead, they should be removed for kinematic mode.

NOTE: This assumes we won't switch back to dynamic mode later (JointMotors would need to be re-created)

How Has This Been Tested

Locally via benchmark.

Types of changes

  • [Bug Fix] (non-breaking change which fixes an issue)

Checklist

  • My code follows the code style of this project.
  • I have updated the documentation if required.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes if required.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 8, 2023
for i, jidx in enumerate(self.params.leg_joints):
self._set_motor_pos(jidx, ctrl[i])
if mt == MotionType.DYNAMIC:
self._set_motor_pos(jidx, ctrl[i])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To confirm, the agent joint position will still update in Kinematic mode? Do we also need to have similar logic for the other articulated objects (like cabinets and fridges)? Or are the motors only instantiated on the robots?

Copy link
Contributor Author

@aclegg3 aclegg3 Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To confirm, the agent joint position will still update in Kinematic mode?

Yes, the line outside the loop:

self.sim_obj.joint_positions = joint_positions

Does the kinematic position setting. The conditional line here sets the motor targets. These motors have already been removed for kinematic mode, so it would error out.

Do we also need to have similar logic for the other articulated objects (like cabinets and fridges)?

The changes in rearrange_sim remove all joint motors for all articulated object instances in kinematic mode. Assuming the scene is already loaded and all the furniture is instantiated, that should remove all joint motors. If that assumption isn't valid we should follow-up.

@aclegg3 aclegg3 merged commit e0c78bc into main Sep 11, 2023
@aclegg3 aclegg3 deleted the remove-kinematic-joint-motors branch September 11, 2023 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants