Skip to content

Rename skgym to skyrl-gym#31

Merged
tyler-griggs merged 5 commits intomainfrom
tgriggs/skygym-rename
Jun 25, 2025
Merged

Rename skgym to skyrl-gym#31
tyler-griggs merged 5 commits intomainfrom
tgriggs/skygym-rename

Conversation

@tyler-griggs
Copy link
Member

@tyler-griggs tyler-griggs commented Jun 25, 2025

  • Top-level path is skyrl-gym
  • Sub-directory (and references in-code such as imports) is skyrl_gym
  • Textual references may need a second pass. Sometimes it is SkyRL-gym and sometimes SkyRL Gym

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved, not deleted.

if __name__ == "__main__":
parser = argparse.ArgumentParser()
parser.add_argument("--output_dir", default="~/data/gsm8k")
parser.add_argument("--output_dir", default="/home/ubuntu/tgriggs/SkyRL/skyrl-train/data/gsm8k")
Copy link
Member

Choose a reason for hiding this comment

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

revert

Copy link
Member Author

Choose a reason for hiding this comment

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

oops

- deepspeed_config@deepspeed_config.train: train
- deepspeed_config@deepspeed_config.eval: eval
- skygym_config@skygym: default
- skyrl_gym_config@skyrl_gym: default
Copy link
Member

Choose a reason for hiding this comment

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

can also just use gym_config because there's only one that is baked in

Copy link
Member Author

Choose a reason for hiding this comment

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

But may as well stick with skyrl_gym? The full name skyrl_gym_config will be used only right here so it's not too big a deal

from skyrl_train.generators.skyrl_gym_generator import SkyRLGymGenerator

return SkyGymGenerator(
return SkyRLGymGenerator(
Copy link
Member

Choose a reason for hiding this comment

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

Ughh this name is nasty

Copy link
Member Author

Choose a reason for hiding this comment

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

My mind has been reading "SkyRL" as pronounced "schiral" and it makes all names sound better XD

Copy link
Member

Choose a reason for hiding this comment

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

There's a recent PR #30 which added "skygym" config to a example. Can you merge from main and search for skygym again

Copy link
Member Author

Choose a reason for hiding this comment

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

yes.

Copy link
Member

@SumanthRH SumanthRH left a comment

Choose a reason for hiding this comment

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

LGTM

@tyler-griggs tyler-griggs merged commit bde56ee into main Jun 25, 2025
3 checks passed
@tyler-griggs tyler-griggs deleted the tgriggs/skygym-rename branch June 28, 2025 16:52
fannie1208 pushed a commit to vinid/SkyRL that referenced this pull request Aug 19, 2025
* Top-level path is `skyrl-gym`
* Sub-directory (and references in-code such as imports) is `skyrl_gym`
* Textual references may need a second pass. Sometimes it is `SkyRL-gym`
and sometimes `SkyRL Gym`
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.

2 participants