Skip to content
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

chore: update trl to grpo_vllm branch, move lighteval to uv #30

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

gerred
Copy link

@gerred gerred commented Jan 25, 2025

  • changes setup.py setup to uv

This is still WIP - I'm going to take the opportunity here to swap from flash-attn to sdpa and remove that dependency, and then I need to verify and push the lockfile.

Additionally, when syncing the exiting lighteval branch with git lfs intalled for the repo, it looks like a blob may not have been pushed?

  × Failed to download and build `lighteval @
  │ git+https://github.com/huggingface/lighteval.git@4f381b352c0e467b5870a97d41cb66b487a2c503`
  ├─▶ Git operation failed
  ╰─▶ process didn't exit successfully: `/usr/bin/git reset --hard
      4f381b352c0e467b5870a97d41cb66b487a2c503` (exit status: 128)
      --- stderr
      Downloading tests/reference_scores/harness_metrics.json (48 MB)
      Error downloading object: tests/reference_scores/harness_metrics.json (e5dffe1):
      Smudge error: Error downloading tests/reference_scores/harness_metrics.json
      (e5dffe1e990e1e839322b74ff02f306ea468ad7602492f62f987cae1bb546b84): error transferring
      "e5dffe1e990e1e839322b74ff02f306ea468ad7602492f62f987cae1bb546b84": [0] remote missing
      object e5dffe1e990e1e839322b74ff02f306ea468ad7602492f62f987cae1bb546b84

      Errors logged to
      '/Users/gerred/.cache/uv/git-v0/checkouts/0c5752dc26bdac93/4f381b3/.git/lfs/logs/20250125T105527.363374.log'.
      Use `git lfs logs last` to view the log.
      error: external filter 'git-lfs filter-process' failed
      fatal: tests/reference_scores/harness_metrics.json: smudge filter lfs failed

@gerred gerred marked this pull request as ready for review January 25, 2025 17:11
@gerred
Copy link
Author

gerred commented Jan 25, 2025

@lewtun This will break the GH action at the moment, was leaving it for the other contributor to come through, move the action, and setup with ruff for the whole thing, but it syncs and we're ready. Once I get over into TRL I'll swap over to sdpa, but as long as it's from the wheel, flash-attn syncs just fine at the moment.

@lewtun
Copy link
Member

lewtun commented Jan 25, 2025

This awesome, thanks a lot! Would you mind updating the installation instructions in the README?

@gerred
Copy link
Author

gerred commented Jan 25, 2025

yep! I just went from fresh clone to a PPO test run and worked out a few things along the way, will do one more pass on pyproj and readme and ping


[tool.uv.sources]
transformers = { git = "https://github.com/huggingface/transformers.git", branch = "main" }
trl = { git = "https://github.com/huggingface/trl.git", branch = "grpo_vllm" }
Copy link
Member

Choose a reason for hiding this comment

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

Use main branch here

Copy link
Author

@gerred gerred Jan 25, 2025

Choose a reason for hiding this comment

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

copy, I'm doing uv pip install -e . from that venv (which I'll document) in my trl clone after I uv sync, so that makes more sense anyway.

@ygdrax
Copy link
Contributor

ygdrax commented Jan 27, 2025

ill break the GH action at the moment, was leaving it for the other contributor to come through, move the action, and setup with ruff for the whole thing, but it syncs and we're ready. Once I get over into TRL I'll swap over to sdpa, but as long as it's from the wheel, flash-attn syncs just fine at the moment.

I can pick the creation of GA with the new setup.

@gerred
Copy link
Author

gerred commented Jan 28, 2025

OK, sorry for the delay - I wanted to make sure it all built and compiled and then things could start up, and I lost a PCIe slot apparently that was throwing issues. I'm clear on this and will tie it together this evening once I have a few other things off my plate!

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.

4 participants