-
Notifications
You must be signed in to change notification settings - Fork 496
Qwen2.5 VL GRPO notebook #61
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
base: main
Are you sure you want to change the base?
Conversation
@rolandtannous Could you check if this notebook works as expected thanks :) |
on it |
@GAD-cell @danielhanchen
The error is as shown in this screenshot: This is caused by the Transformers library version installed in the colab environment. 2- If I downgrade the transformers version to 4.52.4 by forcing the version in the install cell as follows:
instead of
the error disappears, but then the notebook throws another cuda device assert runtime exception in the compiled compute_loss method , on both colab T4 and colab A100, during GRPO training. The runtime error is shown in the following screenshot Not sure if this is due to some code that was committed and merged to unsloth while unslothai/unsloth#2752 was being worked on, but it's definitely worth revisiting . 3- Not very trivial but should be fixed. reference to '/content/' library can lead to permissions error on local systems where the user requires sudo or isn't root in this cell
one way to solve this, is to replace '/content/' with './content' or '~/content' |
Oh, thank you for the review! It was working last week, so you're right, it must be a recent update that broke the notebook. |
Hey @rolandtannous. |
@GAD-cell do you mind pushing the additional changes you made to the same 2752 PR? unslothai/unsloth#2752, that way we have the updated modifications in one consolidated file, and make sure there are no potential conflicts. Just switch locally to your 2752 branch, add the changes from PR188, then recommit and push 2752. This will update your PR2752. Then close PR188. Once that's done i'll go ahead and test. |
Sorry maybe it wasn't clear but PR188 is for unsloth_zoo and PR2752 for unsloth so I can't push in the same PR. |
you're right. completely missed that one. slow morning. |
@rolandtannous oh, okay, sorry about that. I just ran the notebook on a new VM and couldn’t reproduce your error with my updated code on colab-A100 and colab-T4. So maybe your code didn't update correctly ? If you used the same vm, don't forget to remove unsloth_compiled_cache. I'm checking again to see if I missed something. Yes, I do have an updated fork, just run these commands at the beginning of the notebook instead of the regular installation: ! pip install -U git+https://github.com/GAD-cell/unsloth.git@VLM_GRPO
! pip install -U git+https://github.com/GAD-cell/unsloth-zoo.git@VLM_GRPO And it should work ( and also run the cell 'extra colab install' right after ). |
hello @GAD-cell I verified that the code updated correctly by examining the in-place files after running the installation cells. I don't think that's the issue.
You're overwriting a lot of packages on the colab environment. It results in installation taking more time. |
@rolandtannous ok yes my bad for --no-deps. |
@GAD-cell it works installing from his branch with --no-deps both on a T4, and A100 colabs If someone wants to reproduce. These are the updated installed cells I used
and
note the --force-reinstall to ensure that the install is overwritten by the version with the fixes @danielhanchen confirmed to work now. can be merged Thank you for your contribution ! |
Added a notebook for VLM GRPO. (I think I've corrected all the spelling error also).
Works along with PR 2752
@danielhanchen