Skip to content

Conversation

@varun-sundar-rabindranath
Copy link
Contributor

@varun-sundar-rabindranath varun-sundar-rabindranath commented Jul 25, 2025

Purpose

Documentation for FusedMoEModularKernel

Test Plan

None

Test Result

None

@github-actions
Copy link

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

@mergify mergify bot added the documentation Improvements or additions to documentation label Jul 25, 2025
@varun-sundar-rabindranath varun-sundar-rabindranath marked this pull request as draft July 25, 2025 16:36
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 adds documentation for the FusedMoE Modular Kernel. The documentation is comprehensive and provides a good overview of the architecture. I've identified a few high-impact issues to improve clarity and correctness: there are some recurring typos in key class names, and a couple of tables are not formatted correctly in Markdown, which hinders readability. The suggested changes should address these points.

@varun-sundar-rabindranath varun-sundar-rabindranath marked this pull request as ready for review July 25, 2025 20:34
@varun-sundar-rabindranath
Copy link
Contributor Author

@bnellnm PTAL! Thanks 🙌

Comment on lines 143 to 147
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should elaborate on this section for a bit.

@bnellnm
Copy link
Collaborator

bnellnm commented Jul 25, 2025

I think there should be a brief description of the contiguous vs. batched formats at the beginning. I don't think it's obvious what the batched format is unless you've been working on MoE stuff already.

@varun-sundar-rabindranath
Copy link
Contributor Author

I think there should be a brief description of the contiguous vs. batched formats at the beginning. I don't think it's obvious what the batched format is unless you've been working on MoE stuff already.

Done 👍 Please take a look 🙌

Comment on lines +167 to +170
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's important to note that layer.py sets self.fused_experts to the fused moe object and that the subclass needs to use this in it's own apply method.

Copy link
Collaborator

@bnellnm bnellnm left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! Just one final comment about init_prepare_finalize.

@hmellor
Copy link
Member

hmellor commented Jul 28, 2025

Here's the rendered page preview https://vllm--21623.org.readthedocs.build/en/21623/design/fused_moe_modular_kernel.html

If you use a level 1 heading in the MD I believe the title will be capitalised properly

@varun-sundar-rabindranath
Copy link
Contributor Author

Here's the rendered page preview https://vllm--21623.org.readthedocs.build/en/21623/design/fused_moe_modular_kernel.html

If you use a level 1 heading in the MD I believe the title will be capitalised properly

@hmellor done 👍 Please let me know if you see more issues. Thanks.

@hmellor
Copy link
Member

hmellor commented Jul 28, 2025

Sorry, I meant if you title it

# Fused MoE Modular Kernels

Varun Sundar Rabindranath added 11 commits July 29, 2025 12:47
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Copy link
Member

@hmellor hmellor left a comment

Choose a reason for hiding this comment

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

Here is the rendered preview: https://vllm--21623.org.readthedocs.build/en/21623/design/fused_moe_modular_kernel.html. Lots of things aren't rendering correctly

Signed-off-by: Varun Sundar Rabindranath <[email protected]>
@varun-sundar-rabindranath
Copy link
Contributor Author

Thanks @hmellor - Sorry you had to go through the entire documents. I believe things should be fixed now.

Signed-off-by: Varun Sundar Rabindranath <[email protected]>
@hmellor
Copy link
Member

hmellor commented Jul 29, 2025

Thanks for making those fixes, I've replied in the thread about the !!! note syntax

Signed-off-by: Varun Sundar Rabindranath <[email protected]>
@simon-mo simon-mo merged commit f03e9cf into vllm-project:main Jul 29, 2025
5 of 8 checks passed
liuyumoye pushed a commit to liuyumoye/vllm that referenced this pull request Jul 31, 2025
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Co-authored-by: Varun Sundar Rabindranath <[email protected]>
x22x22 pushed a commit to x22x22/vllm that referenced this pull request Aug 5, 2025
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Co-authored-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: x22x22 <[email protected]>
npanpaliya pushed a commit to odh-on-pz/vllm-upstream that referenced this pull request Aug 6, 2025
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Co-authored-by: Varun Sundar Rabindranath <[email protected]>
jinzhen-lin pushed a commit to jinzhen-lin/vllm that referenced this pull request Aug 9, 2025
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Co-authored-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Jinzhen Lin <[email protected]>
noamgat pushed a commit to noamgat/vllm that referenced this pull request Aug 9, 2025
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Co-authored-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Noam Gat <[email protected]>
paulpak58 pushed a commit to paulpak58/vllm that referenced this pull request Aug 13, 2025
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Co-authored-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Paul Pak <[email protected]>
diegocastanibm pushed a commit to diegocastanibm/vllm that referenced this pull request Aug 15, 2025
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Co-authored-by: Varun Sundar Rabindranath <[email protected]>
Signed-off-by: Diego-Castan <[email protected]>
epwalsh pushed a commit to epwalsh/vllm that referenced this pull request Aug 28, 2025
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Co-authored-by: Varun Sundar Rabindranath <[email protected]>
zhewenl pushed a commit to zhewenl/vllm that referenced this pull request Aug 28, 2025
Signed-off-by: Varun Sundar Rabindranath <[email protected]>
Co-authored-by: Varun Sundar Rabindranath <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation force-merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants