Skip to content

Conversation

@Datta0
Copy link
Collaborator

@Datta0 Datta0 commented Dec 1, 2025

Also sync along with unslothai/unsloth#3662

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @Datta0, 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 focuses on enhancing the robustness and compatibility of the system, particularly concerning vLLM and LoRA integrations. It introduces a more flexible approach to loading LoRA adapters, ensuring proper handling of extended vocabularies across different API versions. Additionally, it refines the parameter handling for sampling functions and cleans up an obsolete patch related to vLLM's sampling parameters, streamlining the codebase.

Highlights

  • LoRA Adapter Loading Improvement: Refactored the LoRA adapter loading mechanism to dynamically handle different API signatures for vocabulary size and embedding padding, ensuring better compatibility and correct application of lora_extra_vocab_size.
  • Sampling Parameter Filtering: Implemented filtering for generation_kwargs in grpo_update_SamplingParams to ensure only valid sampling parameters are passed, preventing potential errors from unrecognized arguments.
  • VLLM Patch Removal: Removed an outdated temporary patch that mapped GuidedDecodingParams to StructuredOutputsParams in vLLM, indicating that the underlying dependency issue has been resolved.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a fix for vLLM LoRA extra vocabulary handling by adding backward compatibility for a parameter name change in vllm_lora_worker_manager.py. The change is well-implemented and improves code maintainability by reducing duplication. Additionally, the PR includes several stylistic cleanups by removing trailing whitespace and also removes a temporary patch from vllm_utils.py, which is a welcome cleanup. I have one suggestion in rl_replacements.py to make a code block more concise and Pythonic.

Comment on lines +186 to +190
new_generation_kwargs = {}
for key in generation_kwargs.keys():
if key in good_sampling_params_keys:
new_generation_kwargs[key] = generation_kwargs[key]
generation_kwargs = new_generation_kwargs
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This loop for filtering generation_kwargs can be made more concise and Pythonic by using a dictionary comprehension. This improves readability and is generally more performant.

Suggested change
new_generation_kwargs = {}
for key in generation_kwargs.keys():
if key in good_sampling_params_keys:
new_generation_kwargs[key] = generation_kwargs[key]
generation_kwargs = new_generation_kwargs
generation_kwargs = {k: v for k, v in generation_kwargs.items() if k in good_sampling_params_keys}

@danielhanchen danielhanchen merged commit 89d710c into unslothai:nightly Dec 1, 2025
danielhanchen added a commit that referenced this pull request Dec 1, 2025
* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Bug fixes

* Update patching_utils.py

* Update patching_utils.py

* Update patching_utils.py

* Update rl_replacements.py

* Update patching_utils.py

* Update patching_utils.py

* Update patching_utils.py

* flash attn

* Update gpt_oss.py

* Update __init__.py

* Update attention_sink.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* dropout_p

* Update gpt_oss.py

* Update gpt_oss.py

* Update attention_sink.py

* Update gpt_oss.py

* Update gpt_oss.py

* fix

* Update attention_sink.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update loss_utils.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update loss_utils.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Update gpt_oss.py

* Versioning

* Update saving_utils.py

* Update saving_utils.py

* Update saving_utils.py

* Update saving_utils.py

* Update saving_utils.py

* Update saving_utils.py

* Update saving_utils.py

* Update saving_utils.py

* Fix Gemma 3

* Update misc.py

* Update rl_environments.py

* Update pyproject.toml

* Update rl_environments.py

* Update __init__.py

* Update empty_model.py

* Update empty_model.py

* Update empty_model.py

* Update empty_model.py

* Device type

* Update vllm_utils.py

* Update compiler.py

* Update empty_model.py

* Update vllm_utils.py

* Update empty_model.py

* Fixes

* Update empty_model.py

* Update empty_model.py

* Update __init__.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update rl_environments.py

* Update cross_entropy_loss.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update rl_environments.py

* Update vllm_utils.py

* Qwen3 VL vLLM (#324)

* qwen3 vl additional layers

* qwen3 fused vision qkv

* refactor for handling qwen 3 vl

* [WIP] fix backward pass issues

* out hidden size change

* Qwen 2.5 and qwen 3 conv3d->Linear vLLM changes

* Update __init__.py

* Update __init__.py

* Update __init__.py

* Update __init__.py

* Update __init__.py

* Update __init__.py

* Update __init__.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update pyproject.toml

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update __init__.py

* Update compiler.py

* Update __init__.py

* Update vllm_utils.py

* Update rl_replacements.py

* Update rl_replacements.py

* Update rl_replacements.py

* Fix CE compile

* Update loss_utils.py

* Update cross_entropy_loss.py

* Fix

* Deepseekocr fix: save single model shard (#346)

* DeepSeekOCR Fix: check for saftensors_list shard naming convention

* turned off shard padding length check bc deepseeks padding is different

* if you try to copy the index.json file and the same file alredy exists it wil throw and error.

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update patching_utils.py

* Fp8 compressed (#358)

* [WIP] compressed tensors support for FP8

* [WIP 2/n] improve loading fake layer

* [WIP 3/n] improve loading fake layer

* [WIP 4/n] improve loading fake layer

* revert seq and token util calculation

---------

Co-authored-by: Datta Nimmaturi <venkatadattasainimmaturi@gmail.com>

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update gradient_checkpointing.py

* Update gpt_oss.py

* Update gpt_oss.py

* updates for vLLM compativility with lora (#359)

* updates for vLLM compativility with lora

* LoRA extra vocab cleanup

---------

Co-authored-by: Datta Nimmaturi <venkatadattasainimmaturi@gmail.com>

* Update vllm_utils.py

* Versioning

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Update vllm_utils.py

* Fix

* Qwen3MoE

* Update qwen3_moe.py

* Update qwen3_moe.py

* LoRA extra vocab fix (#367)

* Update qwen3_moe.py

* Update qwen3_moe.py

* Update qwen3_moe.py

* Update qwen3_moe.py

* Update __init__.py

---------

Co-authored-by: Datta Nimmaturi <venkatadattasainimmaturi@gmail.com>
Co-authored-by: DoubleMathew <mmathew23@gmail.com>
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