Remove legacy skyrl-train and skyrl-tx folders#1639
Open
taivu1998 wants to merge 1 commit into
Open
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request completes the repository reorganization by removing the legacy skyrl-train and skyrl-tx README files and updating the main README.md to reflect the unified skyrl/ package structure. Feedback was provided to improve the readability of the component descriptions in the README.md by using sub-bullets to clearly distinguish between the training and Tinker/TX stack paths.
|
|
||
| * [`skyrl-train`](./skyrl-train): A modular, performant training framework for RL. | ||
| * [`skyrl-tx`](./skyrl-tx): A cross-platform library implementing a backend for the [Tinker API](https://docs.skyrl.ai/docs/tinker/overview), with a unified engine for training and inference. | ||
| - [skyrl](./skyrl): Our unified library for RL on your own hardware, with support for the [Tinker API](https://docs.skyrl.ai/docs/tinker/overview). The training stack lives in [`skyrl/train`](./skyrl/train) and [`skyrl/backends/skyrl_train`](./skyrl/backends/skyrl_train), while the Tinker/TX stack lives in [`skyrl/tinker`](./skyrl/tinker), [`skyrl/tx`](./skyrl/tx), and [`skyrl/backends`](./skyrl/backends). |
Contributor
There was a problem hiding this comment.
The updated description for the skyrl component is quite dense and difficult to scan. Breaking it down into sub-bullets (similar to the previous structure) would improve readability and help users quickly identify the relevant paths for the training vs. Tinker/TX stacks.
Suggested change
| - [skyrl](./skyrl): Our unified library for RL on your own hardware, with support for the [Tinker API](https://docs.skyrl.ai/docs/tinker/overview). The training stack lives in [`skyrl/train`](./skyrl/train) and [`skyrl/backends/skyrl_train`](./skyrl/backends/skyrl_train), while the Tinker/TX stack lives in [`skyrl/tinker`](./skyrl/tinker), [`skyrl/tx`](./skyrl/tx), and [`skyrl/backends`](./skyrl/backends). | |
| - [skyrl](./skyrl): Our unified library for RL on your own hardware, with support for the [Tinker API](https://docs.skyrl.ai/docs/tinker/overview). | |
| * **Training stack**: [`skyrl/train`](./skyrl/train) and [`skyrl/backends/skyrl_train`](./skyrl/backends/skyrl_train) | |
| * **Tinker/TX stack**: [`skyrl/tinker`](./skyrl/tinker), [`skyrl/tx`](./skyrl/tx), and [`skyrl/backends`](./skyrl/backends) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
skyrl-train/andskyrl-tx/folders.skyrl/package and the current training, Tinker, TX, and backend paths.Motivation
Issue #1519 asks to clean up the old
skyrl-trainandskyrl-txfolders. On currentmain, those folders only contained README stubs from the repository reorganization, while the actual implementation already lives underskyrl/. Keeping the old root folders around makes the repository look like it still has separate package entrypoints and leaves stale local links in the README.User Impact
Users now land on the canonical unified package layout from the root README:
skyrl/trainandskyrl/backends/skyrl_trainskyrl/tinker,skyrl/tx, andskyrl/backendsThis is a cleanup-only change; it does not rename or remove the active Python modules, extras, CI jobs, or backend concepts that still intentionally use the SkyRL-Train/TX names.
Validation
git diff --checkCloses #1519.