-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[doc] chore: improve docstring consistency in device.py #4691
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
[doc] chore: improve docstring consistency in device.py #4691
Conversation
Improves docstring quality in verl/utils/device.py following Google-style format which is already the dominant style in the codebase. Changes: - Added proper Returns sections with types to all functions - Added missing docstrings to auto_set_ascend_device_name() and get_device_capability() - Improved clarity and consistency of existing docstrings - Fixed type annotation on get_torch_device() (removed invalid `any`) This is part of the effort to improve docstring consistency across the verl codebase as described in verl-project#1345. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request improves docstring consistency in verl/utils/device.py by adding and updating docstrings to follow the Google-style format, and fixing an invalid type hint. The changes significantly improve code readability and documentation. I've provided a couple of suggestions to further improve type hint correctness, which will enhance type safety and maintainability.
|
|
||
| def get_torch_device() -> any: | ||
| """Return the corresponding torch attribute based on the device type string. | ||
| def get_torch_device(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While removing the invalid any type hint is a good fix, it would be even better to provide a correct type hint for this function to improve type safety and code clarity. Since this function returns a module, you could use Any from the typing module or types.ModuleType from the types module.
For example:
from typing import Any
def get_torch_device() -> Any:
# ...This would make the function's return type explicit.
verl/utils/device.py
Outdated
| config.trainer.device = "npu" | ||
|
|
||
|
|
||
| def get_device_capability(device_id: int = 0) -> tuple[int, int]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type hint tuple[int, int] is incorrect. As the docstring correctly states, when CUDA is not available, this function returns (None, None). The type hint should be tuple[int | None, int | None] to reflect that the values can be None.
| def get_device_capability(device_id: int = 0) -> tuple[int, int]: | |
| def get_device_capability(device_id: int = 0) -> tuple[int | None, int | None]: |
|
@yurekami Please resolve conflict |
- Add return type annotation to get_torch_device() -> types.ModuleType - Add type annotation to config parameter in auto_set_ascend_device_name() - Fix get_device_capability return type to allow None values 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Part of the effort to improve docstring consistency across the verl codebase as described in #1345.
This PR focuses on
verl/utils/device.pywhich had several functions with incomplete or missing docstrings.Changes:
auto_set_ascend_device_name()get_device_capability()get_torch_device()(removed invalidanytype hint)Style Note
Based on investigation of the codebase, Google-style docstrings are already the most commonly used format. This PR follows that convention rather than NumPy style, to maintain consistency with existing code.
Test plan
🤖 Generated with Claude Code