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

Greatly Improved Interfaces + Bug Fixes #137

Merged
merged 52 commits into from
Dec 18, 2020
Merged

Greatly Improved Interfaces + Bug Fixes #137

merged 52 commits into from
Dec 18, 2020

Conversation

cremebrule
Copy link
Member

@cremebrule cremebrule commented Dec 5, 2020

Overview

While most surface-level functionality hasn't changed, the underlying infrastructure has been heavily reworked to reduce redundancy, improve standardization and ease-of-usage, and future-proof against expected expansions. Specifically, the following standards were pursued:

  • Pretty much everything should have a name (no name = no reference in sim)
  • All models should have a standardized interface (MujocoModel)
  • Any manipulation-specific properties or methods should be abstracted away to a subclass to future-proof against novel robotic domains that might be added in the future.
  • All associated attributes should try to be kept to a single object reference, to prevent silent errors from occurring due to partially modified objects. For example, instead of having self.object and self.object_name, just have self.object, since it already includes its own name reference in self.object.name.

Tests conducted

  • All demos work

  • All scripts work

  • All tests pass

Highlighted Features and Changes

This is not an exhaustive list, but includes the key features / changes in this PR most relevant to the common user that should greatly streamline environment prototyping and debugging.

Standardized Model Class Hierarchy: Now, all (robot, gripper, object) models inherit from the MujocoModel class, which defines many useful properties and methods, including references to the model joints, contact geoms, important sites, etc. This allows much more standardized usage of these models when designing environments.

Modularized Environment Class Hierarchy: We do not expect robosuite to remain solely manipulation-based. Therefore, all environment properties and methods common to manipulation-based domains were ported to ManipulationEnv, allowing future robot task domains to be added with little reworking. Similarly, common properties / methods common to Single or TwoArm environments were ported to SingleArmEnv and TwoArmEnv, respectively. This both (a) removes much redundant code between top-level env classes, and (b) frees users to focus exclusively on the environment prototyping unique to their use case without having to duplicate much boilerplate code. So, for example, Lift now has a class hierarchy of MujocoEnv --> RobotEnv --> ManipulationEnv --> SingleArmEnv --> Lift. Note that similar changes were made to the Robot and RobotModel base classes.

Standardized and Streamlined Object Classes: All object classes now are derived from MujocoObject, which itself is a subclass of MujocoModel. This standardizes the interface across all object source modalities (Generated vs. XML based), and provides the user with an expected set of properties that can be leveraged when prototyping custom environments. Additionally, complex, procedural object generation has been added with the CompositeObject class, of now which the HammerObject and PotWithHandles object are now subclasses of (as examples of how to design custom composite objects).

Standardized Geom Groups: All collision geoms now belong to group 0, while visual geoms belong to group 1. This means that methods can automatically check for the geom type by polling it's group attribute from its element or during sim. Moreover, all collision geoms are assigned solid rgba colors based on their semantic role (e.g.: robot vs. gripper vs. arena vs. objects). If rendering onscreen, you can easily toggle visualizing the visual and collision geoms by pressing 1 or 0, respectively. This can be useful for debugging environments and making sure collision bodies are formed / interacting as expected.

High-Utility Methods for Environment Prototyping: Because of this improved structure, many methods can now take advantage of this standardization. Some especially relevant methods are discussed briefly below:

  • env.get_contacts(model) (any env): This method will return the set of all geoms currently in contact with the inputted model. This is useful for debugging environments, or checking to see if certain conditions are met when designing rewards / interactions.

  • env._check_grasp(gripper, object_geoms) (only manipulation envs): This method will return True if the inputted gripper is grasping the object specified by object_geoms, which could be a MujocoModel or simply a list of geoms that define the object. This makes it very easy to design environments that depend on certain grasping requirements being met.

  • env._gripper_to_target(gripper, target, ...) and env._visualize_gripper_to_target(gripper, target, ...) (only manipulation envs): Methods to help streamline getting relevant distance info between a gripper gripper and target. Target can be a MujocoModel or any specific element (body, geom, site) name. The former calculates the distance, while the latter will set the gripper eef site sphere's color to be proportional to the distance to target. Both are useful for environment prototyping and debugging.

  • model.set_sites_visibility(sim, visible) (any MujocoModel): This method will set all the sites belonging to model in the current sim to either be visible or not depending on the visible arg. This is useful for quick debugging or teleoperation, to aid the user in visualizing specific points of reference in sim.

Other Infrastructure Changes

The following briefly describes other changes made that were significant but not necessarily as directly relevant to the common user. Again, this is not an exhaustive list, but a highlighted list of changes.

  • MountModel class added; pedestals used by robots are now assigned to this class and added to a RobotModel in a similar fashion to how the GripperModel is added.

  • Greater fine-grained control over sites being visualized during environment initialization -- now, can specify gripper_visualizations, robot_visualizations, and env_visualization.

  • Added openGL and openCV image convention option

  • Added macros.py in robosuite.utils and single file to store all macros for our repo. This includes numba macros and now includes domain randomization and image convention macros. Users can modify these macros mid-script by importing the macros module and modifying the module-level vars directly.

  • Placement samplers were no longer belong to Task class, but are separate. This is more intuitive, and allows for more modularity when designing future Task subclasses. Moreover, the placement sampler classes were refactored for more intuitive usage.

  • Tuned / debugged robot models; grasps are now inferred correctly so robot cannot "cheat" a grasping-based reward

  • Refactor all top-level environments in a standardized fashion

  • Add functionality to modify cameras from Arena class; tuned cameras for Door, TwoArmHandover/PegInHole tasks

  • Renamed / modified a bunch of stuff so it's more semantically accurate / intuitive

  • Fixed misc. bugs, including (implicitly) Running self._check_gripper_contact() results in AttributeError: 'SingleArm' object has no attribute 'arm_type' #119

  • Improve documentation

…lors for collision geoms, refactor generated / xml object parsing, fix naming conventions in two_arm_lift
…ve redundancies, fix misc. bugs, standardize all envs and class hierarchy
…ujocoXMLModel class, standardize default materials / textures, standardize model properties
… materials for DR, verify demo scripts work, fix small bugs
…d functionality for modifying cameras in arena
@yukezhu
Copy link
Member

yukezhu commented Dec 6, 2020

Thanks for this big effort! Here are some comments to start with. I will follow up on this as more tests are done.

  1. Too many knobs for visualization options in environment class. What's the key difference between gripper_visualizations and robot_visualizations - do we need both? And what about use_indicator_object and _visualize_gripper_to_target? As these functionalities are mainly used for teleoperation, would it makes them in a teleoperation wrapper rather than the main RobotEnv class?

  2. Plural vs singular nouns. Some functions use singulars gripper in their names, but some might make changes to multiple grippers, e.g., visualize_gripper changes two grippers in bimanual tasks? Should we call visualize_grippers instead? Similarly, inconsistency between singular and plural nouns, like _get_observation in Environments versus get_observations in Robots.

  3. Versioning. Update version number in robosuite/__init__.py to 1.1.0

  4. Control frequency. Is this intended to set the default control_freq to 10? Is this what we used for benchmarking?

  5. What's the effect of setting macros.USING_DOMAIN_RANDOMIZATION to True when using domain randomization? Setting it to False appears to be just fine in the demo script. We might need a warning if the flag is required when using the wrapper.

  6. Consider breaking down generated_objects.py into separate files. Maybe it can contain PrimitiveObject and CompositeObject and the other procedurally generated objects can be put in separate files in a subfolder. This makes it cleaner to add new procedural objects in the future.

svd_u, svd_s, svd_v = np.linalg.svd(lambda_ori_inv)
svd_s_inv = np.array([0 if x < singularity_threshold else 1. / x for x in svd_s])
lambda_ori = svd_v.T.dot(np.diag(svd_s_inv)).dot(svd_u.T)
# take the inverses, but zero out small singular values for stability
Copy link
Member

Choose a reason for hiding this comment

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

Is this comment deprecated with the new change of using pinv?

Copy link
Member

Choose a reason for hiding this comment

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

technically I believe pinv will use the SVD and ignore singular values that are smaller than some threshold - that's why I put this comment

Copy link
Member

Choose a reason for hiding this comment

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

take a look at the rcond argument here

Copy link
Member

Choose a reason for hiding this comment

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

Ok cool

@yukezhu
Copy link
Member

yukezhu commented Dec 14, 2020

Addressed issue #123

Copy link
Member

@yukezhu yukezhu left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants