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

[RLlib][Docs] Added RLModule user-guide to the docs #33909

Merged
merged 29 commits into from
Apr 7, 2023

Conversation

kouroshHakha
Copy link
Contributor

Why are these changes needed?

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Signed-off-by: Kourosh Hakhamaneshi <[email protected]>
@kouroshHakha
Copy link
Contributor Author

@avnishn In the beginning of the doc I allude to your migration guide. So that need to get merged in for this make sense. Your doc will be cross linked to this one.

@kouroshHakha
Copy link
Contributor Author

@ArturNiederfahrenhorst In the end of this doc I allude to the catalog user-guide (in the extension of existing RLModules section). You need to write your use-guide and then we will link it here.

@ArturNiederfahrenhorst
Copy link
Contributor

@kouroshHakha sg

@ollie-iterators
Copy link

/ray/doc/source/rllib/rllib-rlmodule.rst:403: WARNING: Inline interpreted text or phrase reference start-string without end-string.

Copy link
Member

@avnishn avnishn left a comment

Choose a reason for hiding this comment

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

publishing for now

doc/source/rllib/rllib-rlmodule.rst Outdated Show resolved Hide resolved
doc/source/rllib/rllib-rlmodule.rst Outdated Show resolved Hide resolved
Copy link
Member

@avnishn avnishn left a comment

Choose a reason for hiding this comment

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

leaving additional comments from offline convor

doc/source/rllib/rllib-rlmodule.rst Show resolved Hide resolved
kouroshHakha and others added 6 commits March 30, 2023 13:16
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Signed-off-by: Kourosh Hakhamaneshi <[email protected]>
Signed-off-by: Kourosh Hakhamaneshi <[email protected]>
kouroshHakha and others added 12 commits April 1, 2023 10:56
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: kourosh hakhamaneshi <[email protected]>
Signed-off-by: Kourosh Hakhamaneshi <[email protected]>
@richardliaw
Copy link
Contributor

Hey, we should be testing codeblocks in CI and not using code-blocks. Can you fix? You can take a look at Tune for an example of what to do.


Enable RLModules by setting the ``_enable_rl_module_api`` flag to ``True`` in the configuration object.

.. code-block:: python
Copy link
Contributor

Choose a reason for hiding this comment

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

like Richard mentioned, let's make all these literalinclude statements. Otherwise these code blocks risk being outdated quickly. RLlib already has a doc_code folder for that purpose. Alternatively, use an example or test folder in rllib, but make sure it's all tested. Thanks!

@kouroshHakha kouroshHakha added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Apr 7, 2023
@gjoliver gjoliver merged commit a39c933 into ray-project:master Apr 7, 2023
kouroshHakha added a commit to kouroshHakha/ray that referenced this pull request Apr 7, 2023
jjyao pushed a commit that referenced this pull request Apr 17, 2023
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants