Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for LoRA with NVFP4 Mixture-of-Experts models. The changes achieve this by forcing the selection of the MARLIN backend when LoRA is enabled for NVFP4 MoE, as it provides the necessary moe_sum method for LoRA injection. A warning is correctly logged to inform the user about this backend selection. Additionally, the pull request includes a robustness improvement in vllm/lora/layers/fused_moe.py to handle both positional and keyword arguments when accessing parameters in a decorated function, which prevents potential KeyError exceptions for optional arguments. The changes appear correct and well-implemented.
604e979 to
adad1e6
Compare
54e09c3 to
418a9f1
Compare
418a9f1 to
e49d1f9
Compare
pavanimajety
left a comment
There was a problem hiding this comment.
LGTM, thanks! What's needed from Flashinfer or other experts backends to support this feature?
Signed-off-by: Daniel Serebrenik <daserebrenik@nvidia.com>
e49d1f9 to
5e1a105
Compare
|
@pavanimajety as far as I understand, CUTLASS requires changes in flashinfer to support LoRA (same for cutlass with FP8 models). And for As far as I can see - most quantizations support LoRA only with Triton MoE (or Marlin for NVFP4). |
| ) | ||
| else: | ||
| assert isinstance(m_fused_moe_fn.fused_experts, TritonExperts) | ||
| # MarlinExperts used for NVFP4 (see select_nvfp4_moe_backend) |
There was a problem hiding this comment.
remove comment
|
I agree this adds NVFP4 kernel support for LoRA (in the same way it is added for FP8), but I consider how we do this for FP8 to be a hack. Instead, we should add an attribute to the Kernels Could you update the code to add this feature? Im okay if we want to merge the NVFP4 first, assuming you agree to make the follow up to improve the structure |
|
@robertgshaw2-redhat it's not urgent to merge this PR. We can follow the structure you suggested (I would have to review the code areas you mentioned to understand the effort required). |
|
This pull request has merge conflicts that must be resolved before it can be |
Purpose
In current vLLM main there is not support for LoRA with NVFP4 MoE models.
This PR enables LoRA support by using the existing MarlinExperts backend.
Requires this fix:
#34575
Test Plan
Test support for LoRA adapters with Nemotron Nano NVFP4:
https://huggingface.co/nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-NVFP4
LoRA adapters for testing can be found here:
https://huggingface.co/models?other=base_model:adapter:nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16
Test Result
Tested on B200 (requires env-var
VLLM_LORA_DISABLE_PDL=1).Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.