fix: copy numpy arrays from np.frombuffer to fix R3 read-only tensor warning#703
fix: copy numpy arrays from np.frombuffer to fix R3 read-only tensor warning#703NJX-njx wants to merge 1 commit intoradixark:mainfrom
Conversation
…warning Fixes radixark#599 np.frombuffer() returns read-only arrays since they reference the underlying buffer. When these are later converted via torch.from_numpy() in training_utils/data.py, PyTorch warns about non-writable tensors and any writes become undefined behavior. This can cause subtle data corruption in R3 (Rollout Routing Replay). Fix: Add .copy() after np.frombuffer() in both generate_endpoint_utils.py and sglang_rollout.py to ensure the arrays are writable before being passed through the training pipeline.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where NumPy arrays created using Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a PyTorch warning (and potential undefined behavior) triggered when np.frombuffer() returns a read-only NumPy array that is later passed to torch.from_numpy(). The fix applies .copy() immediately after np.frombuffer() in both code paths that handle the routed_experts field from the SGLang inference response, ensuring the resulting array is writable before being converted to a PyTorch tensor in downstream training code.
Changes:
- Added
.copy()afternp.frombuffer(...)insglang_rollout.pyfor the legacy SGLang generate path. - Added
.copy()afternp.frombuffer(...)ingenerate_endpoint_utils.pyfor the newer shared utility path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
miles/rollout/sglang_rollout.py |
Adds .copy() after np.frombuffer() to produce a writable array before reshape() in the legacy generate() function |
miles/rollout/generate_utils/generate_endpoint_utils.py |
Adds .copy() after np.frombuffer() to produce a writable array before reshape() in the shared _get_rollout_topk_from_response() utility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request modifies two files, miles/rollout/generate_utils/generate_endpoint_utils.py and miles/rollout/sglang_rollout.py, to address a potential issue where np.frombuffer returns a read-only array. The change involves adding a .copy() call after np.frombuffer and before .reshape when processing meta_info related to routed_experts or similar data. This ensures that the resulting NumPy array is writable, preventing warnings or undefined behavior when these arrays are subsequently used with torch.from_numpy.
Fixes #599 (partial) - np.frombuffer() returns read-only arrays that cause PyTorch warnings and potential undefined behavior when converted via torch.from_numpy(). Added .copy() in both generate_endpoint_utils.py and sglang_rollout.py.