Skip to content

Latest commit

 

History

History
297 lines (223 loc) · 12.9 KB

CONTRIBUTING.md

File metadata and controls

297 lines (223 loc) · 12.9 KB
Raw

Contributing to Meta-World

Use this guide to prepare your contribution.

What we are looking for in new tasks

We particularly welcome contributions of additional tasks that thematically fit within the MT50 and ML45 benchmarks. Such tasks should conform to the action space, observation space, assets, and reward function form as the other tasks. Further, you must run SAC and PPO on the task and include a learning curve.

Checklist for adding new tasks

Ensure that your task and pull request:

  • Can be performed by a real robot arm
  • Is dissimilar from current tasks
  • Contains meaningful internal variation (e.g. different object positions, etc.)
  • Conforms to the action space, observation space, and reward functions conventions used by metaworld environments
  • Uses existing assets if they exist, and that any new assets added are high-quality
  • Follows the code qualtiy, style, testing, and documentation guidelines outlined below
  • Provides learning curves which show the task can by solved by PPO and SAC, using the implementations linked below

PPO: https://github.com/rlworkgroup/garage/blob/master/src/garage/tf/algos/ppo.py

SAC: https://github.com/rail-berkeley/softlearning

Pull requests

All contributions to the Meta-World codebase are submitted via a GitHub pull request.

Review process

To be submitted, a pull request must satisfy the following criteria:

  1. Rebases cleanly on the master branch
  2. Passes all continuous integration tests
  3. Conforms to the git commit message format
  4. Receives approval from another contributor
  5. Receives approval from a maintainer (distinct from the contributor review)

These criteria may be satisfied in any order, but in practice your PR is unlikely to get attention from contributors until 1-3 are satisfied. Maintainer attention is a scarce resource, so generally maintainers wait for a review from a non-maintainer contributor before reviewing your PR.

Preparing your repo to make contributions

After following the standard Meta-World setup steps, make sure to run to install the pre-commit hooks into your repository. pre-commit helps streamline the pull request process by catching basic problems locally before they are checked by the CI.

To setup pre-commit in your repo:

# make sure your Python environment is activated, e.g.
# conda activate Meta-World
# pipenv shell
# poetry shell
# source venv/bin/activate
pre-commit install -t pre-commit
pre-commit install -t pre-push
pre-commit install -t commit-msg

Once you've installed pre-commit, it will automatically run every time you type git commit.

Code style

The Python code in metaworld conforms to the PEP8 standard. Please read and understand it in detail.

Meta-World specific Python style

These are Meta-World specific rules which are not part of the aforementioned style guides.

  • Python package imports should be sorted alphabetically within their PEP8 groupings.

    The sorting is alphabetical from left to right, ignoring case and Python keywords (i.e. import, from, as). Notable exceptions apply in __init__.py files, where sometimes this rule will trigger a circular import.

  • Prefer single-quoted strings ('foo') over double-quoted strings ("foo").

    Double-quoted strings can be used if there is a compelling escape or formatting reason for using single quotes (e.g. a single quote appears inside the string).

  • Add convenience imports in __init__.py of a package for shallow first-level repetitive imports, but not for subpackages, even if that subpackage is defined in a single .py file.

    For instance, if an import line reads from .foo.bar import Bar then you should add from metaworld.foo.bar import Bar to metaworld/foo/__init__.py so that users may instead write from metaworld.foo import Bar. However, if an import line reads from metaworld.foo.bar.stuff import Baz, do not add from metaworld.foo.bar.stuff import Baz to metaworld/foo/__init__.py, because that obscures the stuff subpackage.

    Do

    metaworld/foo/__init__.py:

    """Foo package."""
from metaworld.foo.bar import Bar

    metaworld/barp/bux.py:

    """Bux tools for barps."""
from metaworld.foo import Bar
from metaworld.foo.stuff import Baz

    Don't

    metaworld/foo/__init__.py:

    """Foo package."""
from metaworld.foo.bar import Bar
from metaworld.foo.bar.stuff import Baz

    metaworld/barp/bux.py:

    """Bux tools for barps."""
from metaworld.foo import Bar
from metaworld.foo import Baz

  • Imports within the same package should be absolute, to avoid creating circular dependencies due to convenience imports in __init__.py

    Do

    metaworld/foo/bar.py

    from metaworld.foo.baz import Baz

b = Baz()

    Don't

    metaworld/foo/bar.py

    from metaworld.foo import Baz  # this could lead to a circular import, if Baz is imported in metaworld/foo/__init__.py

b = Baz()

  • Base and interface classes (i.e. classes which are not intended to ever be instantiated) should use the abc package to declare themselves as abstract.

    i.e. your class should inherit from abc.ABC or use the metaclass abc.ABCMeta, it should declare its methods abstract (e.g. using @abc.abstractmethod) as-appropriate. Abstract methods should all use pass as their implementation, not raise NotImplementedError

    Do

    import abc

class Robot(abc.ABC):
    """Interface for robots."""

    @abc.abstractmethod
    def beep(self):
        pass

    Don't

    class Robot(object):
    "Base class for robots."""

    def beep(self):
        raise NotImplementedError

  • When using external dependencies, use the import statement only to import whole modules, not individual classes or functions.

    This applies to both packages from the standard library and 3rd-party dependencies. If a package has a long or cumbersome full path, or is used very frequently (e.g. numpy, tensorflow), you may use the keyword as to create a file-specific name which makes sense. Additionally, you should always follow the community concensus short names for common dependencies (see below).

    Do

    import collections

import gym.spaces

from garage.tf.models import MLPModel

q = collections.deque(10)
d = gym.spaces.Discrete(5)
m = MLPModel(output_dim=2)

    Don't

    from collections import deque

from gym.spaces import Discrete
import tensorflow as tf

from garage.tf.models import MLPModel

q = deque(10)
d = Discrete(5)
m = MLPModel(output_dim=2)

    Known community-concensus imports

    import numpy as np
import matplotlib.pyplot as plt
import tensorflow as tf
import tensorflow_probability as tfp
import torch.nn as nn
import torch.nn.functional as F
import torch.optim as optim
import dowel.logger as logger
import dowel.tabular as tabular

Documentation

Python files should provide docstrings for all public methods which follow PEP257 docstring conventions and Google docstring formatting. A good docstring example can be found here.

Additional standards:

  • Docstrings for __init__ should be included in the class docstring as suggested in the Google example.
  • Docstrings should provide full type information for all arguments, return values, exceptions, etc. according to the Google format

Application guide

Newly created Python files should follow all of the above standards for docstrings.

Non-trivially modified Python files should be submitted with updated docstrings according to the above standard.

New or heavily-redesigned modules with non-trivial APIs and functionality should provide full text documentation, in addition to docstrings, which covers:

  • Explanation of the purpose of the module or API
  • Brief overview of its design
  • Usage examples for the most common use cases
  • Explicitly calls out common gotchas, misunderstandings, etc.
  • A quick summary of how to go about advanced usage, configuration, or extension

Other languages

Non-Python files (including XML, HTML, CSS, JS, and Shell Scripts) should follow the Google Style Guide for that language

YAML files should use 2 spaces for indentation.

Whitespace (all languages)

  • Use Unix-style line endings
  • Trim trailing whitespace from all lines
  • All files should end in a single newline

Testing

metaworld maintains a test suite to ensure that future changes do not break existing functionality. We use TravisCI to run a unit test suite on every pull request before merging.

  • New functionality should always include unit tests and, where appropriate, integration tests.
  • PRs fixing bugs which were not caught by an existing test should always include a test replicating the bug

Creating Tests

Add a test for your functionality under the metaworld/tests/ directory. Make sure your test filename is prepended with test(i.e. test_<filename>.py) to ensure the test will be run in the CI.

Git

Workflow

metaworld uses a linear commit history and rebase-only merging.

This means that no merge commits appear in the project history. All pull requests, regardless of number of commits, are squashed to a single atomic commit at merge time.

Do's and Don'ts for avoiding accidental merge commits and other headaches:

  • Don't use GitHub's "Update branch" button on pull requests, no matter how tempting it seems
  • Don't use git merge
  • Don't use git pull (unless git tells you that your branch can be fast-forwarded)
  • Don't make commits in the master branch---always use a feature branch
  • Do fetch upstream (rlworkgroup/metaworld) frequently and keep your master branch up-to-date with upstream
  • Do rebase your feature branch on master frequently
  • Do keep only one or a few commits in your feature branch, and use git commit --amend to update your changes. This helps prevent long chains of identical merges during a rebase.

Please see this guide for a tutorial on the workflow. Note: unlike the guide, we don't use separate develop/master branches, so all PRs should be based on master rather than develop

Commit message format

metaworld follows the git commit message guidelines documented here and here. You can also find an in-depth guide to writing great commit messages here

In short:

  • All commit messages have an informative subject line of 50 characters
  • A newline between the subject and the body
  • If relevant, an informative body which is wrapped to 72 characters

Git recipes

These recipes assume you are working out of a private GitHub fork.

If you are working directly as a contributor to rlworkgroup, you can replace references to rlworkgroup with origin. You also, of course, do not need to add rlworkgroup as a remote, since it will be origin in your repository.

Clone your GitHub fork and setup the rlworkgroup remote

git clone git@github.com:<your_github_username>/metaworld.git
cd metaworld
git remote add rlworkgroup git@github.com:rlworkgroup/metaworld.git
git fetch rlworkgroup

Update your GitHub fork with the latest from upstream

git fetch rlworkgroup
git reset --hard master rlworkgroup/master
git push -f origin master

Make a new feature branch and push it to your fork

git checkout master
git checkout -b myfeaturebranch
# make some changes
git add file1 file2 file3
git commit # Write a commit message conforming to the guidelines
git push origin myfeaturebranch

Rebase a feature branch so it's up-to-date with upstream and push it to your fork

git checkout master
git fetch rlworkgroup
git reset --hard rlworkgroup/master
git checkout myfeaturebranch
git rebase master
# you may need to manually reconcile merge conflicts here. Follow git's instructions.
git push -f origin myfeaturebranch # -f is frequently necessary because rebases rewrite history

Release

Modify CHANGELOG.md

For each release in metaworld, modify CHANGELOG.md with the most relevant changes from the latest release. The format is based on Keep a Changelog, which adheres to Semantic Versioning.