Repository navigation
Update installation and import for veRL integration - #58
Conversation
|
Thanks for the contribution! Before we can merge this, we need @StevenShi-23 to sign the Salesforce Inc. Contributor License Agreement. |
There was a problem hiding this comment.
Pull request overview
Adds packaging support for the VERL-based RL integration and prevents package import failures when the (currently missing/WIP) fully-async module isn’t present.
Changes:
- Introduces a new
rloptional dependency group with VERL + supporting libs. - Makes fully-async exports in
mcpuniverse.rl.integrations.verloptional via a guarded import.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pyproject.toml |
Adds an rl extra to install VERL/RL-related dependencies. |
mcpuniverse/rl/integrations/verl/__init__.py |
Wraps fully_async imports/exports in a try/except to avoid import-time failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rl = [ | ||
| "verl>=0.3.0", | ||
| "torch>=2.6.0", | ||
| "ray>=2.0.0", | ||
| "tensordict>=0.6.0", | ||
| "hydra-core>=1.3.0", | ||
| "tqdm>=4.60.0", | ||
| "transformers>=4.40.0", | ||
| "numpy>=1.24.0", | ||
| ] |
There was a problem hiding this comment.
This adds an rl extra in pyproject.toml, but the repo’s main install path documented in README.md is pip install -r requirements.txt, and requirements.txt currently only mentions the vllm/sglang extras. To avoid users still hitting missing-dependency errors for the VERL integration, consider updating the documented installation flow (e.g., mention pip install -e '.[rl]') and/or keeping requirements.txt guidance in sync with this new extra.
| except ImportError: | ||
| pass |
There was a problem hiding this comment.
The except ImportError: pass will also swallow import errors coming from inside fully_async once that module exists (e.g., missing optional deps, syntax errors), making failures hard to debug. Consider only handling the specific case where the submodule is missing (e.g., catch ModuleNotFoundError and verify e.name matches this package’s fully_async module, or pre-check with importlib.util.find_spec), and let other import errors surface.
| except ImportError: | |
| pass | |
| except ModuleNotFoundError as e: | |
| # Only ignore the error if the optional fully_async module itself is missing. | |
| # Re-raise if some other module is missing so real import errors are not hidden. | |
| if e.name not in (__name__ + ".fully_async", "fully_async"): | |
| raise |
…except block and update optional dependencies in pyproject.toml
|
@ChiYeungLaw i've cleaned up the RL installation steps and vllm env var. Please let me know how you think, thanks! |
…dev-ziji Update installation and import for veRL integration
This PR addresses #57 by adding veRL dependencies to project setup script and fixing imports.