Skip to content

[RLlib; docs] Docs do-over (new API stack): ConnectorV2 documentation (part II). #54313

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

Open
wants to merge 35 commits into
base: master
Choose a base branch
from

Conversation

sven1977
Copy link
Contributor

@sven1977 sven1977 commented Jul 3, 2025

Docs do-over (new API stack): ConnectorV2 documentation (part II).

  • Add batch construction phases and formats to generic connector overview page.
  • 3 additional figures illustrating the different batch phases.

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: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
…_redo_connector_v2

Signed-off-by: sven1977 <[email protected]>

# Conflicts:
#	rllib/connectors/common/add_states_from_episodes_to_batch.py
#	rllib/connectors/module_to_env/remove_single_ts_time_rank_from_batch.py
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
…_redo_connector_v2

Signed-off-by: sven1977 <[email protected]>

# Conflicts:
#	doc/source/rllib/package_ref/index.rst
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
sven1977 added 5 commits July 2, 2025 18:29
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
@Copilot Copilot AI review requested due to automatic review settings July 3, 2025 15:03
@sven1977 sven1977 requested review from a team as code owners July 3, 2025 15:03
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR continues the ConnectorV2 documentation overhaul by filling out the generic connector overview with batch phases and adding three illustrative figures, while also refining implementation details to better handle rl_module=None cases and improving fallback logic in algorithm_config.

  • Added "Batch construction phases and formats" section with three new SVG figures in connector-v2.rst.
  • Refined connector implementations (numpy_to_tensor, add_time_dim_to_batch_and_zero_pad, add_states_from_episodes_to_batch) to guard on rl_module=None.
  • Improved environment/action space inference in AlgorithmConfig, fixed typos, and expanded documentation across multiple .rst files.

Reviewed Changes

Copilot reviewed 13 out of 26 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
rllib/examples/connectors/multi_agent_observation_preprocessor.py Clarified multi-agent preprocessor comment.
rllib/connectors/connector_v2.py Fixed backticks around self in docstrings.
rllib/connectors/common/numpy_to_tensor.py Removed unused as_learner_connector and added rl_module=None handling.
rllib/connectors/common/add_time_dim_to_batch_and_zero_pad.py Early-out when rl_module is None before padding.
rllib/connectors/common/add_states_from_episodes_to_batch.py Early-out when rl_module is None before state injection.
rllib/algorithms/algorithm_config.py Corrected typo, unified space inference (env/spaces/self), expanded spec logic.
doc/source/rllib/user-guides.rst Added connector-v2 entry to user guides.
doc/source/rllib/single-agent-episode.rst Updated connector pipeline links to use :ref: roles.
doc/source/rllib/rl-modules.rst Added anchor for custom MultiRLModules.
doc/source/rllib/package_ref/index.rst Included new connector-v2.rst in package reference.
doc/source/rllib/package_ref/connector-v2.rst Added API reference for ConnectorV2.
doc/source/rllib/env-to-module-connector.rst Introduced full env-to-module pipeline docs with figures.
doc/source/rllib/connector-v2.rst Added generic ConnectorV2 overview and batch phases section.
Comments suppressed due to low confidence (1)

rllib/connectors/common/numpy_to_tensor.py:67

  • The docstring still references the removed as_learner_connector parameter; please update the Args section to remove or adjust this entry to match the current signature.
        Args:

@@ -103,7 +103,9 @@
num_envs_per_env_runner=20,
# Define a list of two connector piece to be prepended to the env-to-module
# connector pipeline:
# 1) The custom connector piece (a MultiAgentObservationPreprocessor).
# 1) The custom connector piece: A MultiAgentObservationPreprocessor, which
# enhances each agents' individual observations through adding the
Copy link
Preview

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

Grammar: change "each agents' individual observations" to "each agent's individual observations" for correct possessive singular form.

Suggested change
# enhances each agents' individual observations through adding the
# enhances each agent's individual observations through adding the

Copilot uses AI. Check for mistakes.

@@ -1058,9 +1058,10 @@ def build_env_to_module_connector(

if env is not None:
obs_space = getattr(env, "single_observation_space", env.observation_space)
else:
assert spaces is not None
elif spaces is not None and INPUT_ENV_SINGLE_SPACES in spaces:
Copy link
Preview

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

[nitpick] The logic for inferring obs_space and act_space from env, spaces, or self appears in multiple places; consider extracting this fallback sequence into a shared helper function to reduce duplication and ensure consistency.

Copilot uses AI. Check for mistakes.

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.

1 participant