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

[Doc] Create a new "Usage" section #10827

Merged
merged 9 commits into from
Dec 5, 2024

Conversation

DarkLight1337
Copy link
Member

@DarkLight1337 DarkLight1337 commented Dec 2, 2024

Some of the pages under "Serving" and "Models" aren't really related to their parent section. This PR creates a new "Usage" section to accommodate these pages.

@simon-mo can you help set up redirects as requested in #10428? Done

Copy link

github-actions bot commented Dec 2, 2024

👋 Hi! Thank you for contributing to the vLLM project.
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 do one of these:

  • Add ready label to the PR
  • Enable auto-merge.

🚀

@mergify mergify bot added the documentation Improvements or additions to documentation label Dec 2, 2024
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337 DarkLight1337 requested a review from ywang96 December 2, 2024 11:01
@DarkLight1337
Copy link
Member Author

cc @ywang96 since I have significantly modified the docs for VLMs.

Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
Copy link
Member

@russellb russellb left a comment

Choose a reason for hiding this comment

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

First, thank you for using git mv ! It's really common for people to not do that and accidentally lose the git history of the file.

There are two things I'm thinking about when deciding on groupings of content:

  • who is the audience? (someone running vLLM, someone interacting with it via the API, using it via Python, someone working on the vLLM code, probably others)
  • what problem are they trying to solve?

In this PR, I thought maybe you wanted "Serving" to focus more on "someone running vLLM", but that doesn't seem right. There are deploying docs that stay, but. then the doc on environment variables that a deployer would set got moved.

How would you describe the intended audience and scope of each section?

There are some in the PR that make a lot of sense to move around. A couple examples are "Adding a New Model" and "Enabling Multimodal Inputs". Those are more like "Design" to me. The audience is "people working on the vLLM code," and the content for that audience has been shifted to the end of the docs recently.

@simon-mo
Copy link
Collaborator

simon-mo commented Dec 3, 2024

+1 i think we can expand the scope for broader refactoring of the doc tree, but okay to keep it scoped smaller.

@DarkLight1337
Copy link
Member Author

DarkLight1337 commented Dec 3, 2024

First, thank you for using git mv ! It's really common for people to not do that and accidentally lose the git history of the file.

Actually, I didn't use it. The diffs were small enough that Git was able to detect that automatically 😅 Thanks for telling me about this though.

In this PR, I thought maybe you wanted "Serving" to focus more on "someone running vLLM", but that doesn't seem right. There are deploying docs that stay, but. then the doc on environment variables that a deployer would set got moved.

How would you describe the intended audience and scope of each section?

vLLM supports both offline and online inference. I intend the Usage section to contain docs that apply to both. On the other hand, the Serving section focuses on online inference only.

There are some in the PR that make a lot of sense to move around. A couple examples are "Adding a New Model" and "Enabling Multimodal Inputs". Those are more like "Design" to me. The audience is "people working on the vLLM code," and the content for that audience has been shifted to the end of the docs recently.

I agree that those two pages should be moved elsewhere. It's not really about "Design" though, maybe we can add a new Tutorials section? In any case it's outside the scope of this PR.

@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 3, 2024
@russellb
Copy link
Member

russellb commented Dec 3, 2024

First, thank you for using git mv ! It's really common for people to not do that and accidentally lose the git history of the file.

Actually, I didn't use it. The diffs were small enough that Git was able to detect that automatically 😅 Thanks for telling me about this though.

Oh, I see. GitHub was smart enough to detect it and show it that way in the UI, but git does not see it as a move. Run git log docs/source/usage/compatibility_matrix.rst and you'll see what I mean. The history is a single commit now.

It's up to you if you feel like this is worth fixing.

In this PR, I thought maybe you wanted "Serving" to focus more on "someone running vLLM", but that doesn't seem right. There are deploying docs that stay, but. then the doc on environment variables that a deployer would set got moved.
How would you describe the intended audience and scope of each section?

vLLM supports both offline and online inference. I intend the Usage section to contain docs that apply to both. On the other hand, the Serving section focuses on online inference only.

Yeah, I guess I don't know how to decide what would go under Serving and what would go under Usage.

There are some in the PR that make a lot of sense to move around. A couple examples are "Adding a New Model" and "Enabling Multimodal Inputs". Those are more like "Design" to me. The audience is "people working on the vLLM code," and the content for that audience has been shifted to the end of the docs recently.

I agree that those two pages should be moved elsewhere. It's not really about "Design" though, maybe we can add a new Tutorials section? In any case it's outside the scope of this PR.

That's fine. I actually like how it is now that I look again. All of the content under "Models" groups together nicely.

@DarkLight1337
Copy link
Member Author

DarkLight1337 commented Dec 3, 2024

Oh, I see. GitHub was smart enough to detect it and show it that way in the UI, but git does not see it as a move. Run git log docs/source/usage/compatibility_matrix.rst and you'll see what I mean. The history is a single commit now.

I'm not too bothered by this, since git blame -C still includes previous commits to the original file.

@DarkLight1337
Copy link
Member Author

Does this look good to you now?

@russellb
Copy link
Member

russellb commented Dec 4, 2024

Does this look good to you now?

I'm not opposed to it. Definitely don't let me block it.

I'm still not sure I understand it, though:

Yeah, I guess I don't know how to decide what would go under Serving and what would go under Usage.

but please feel free to proceed if you'd like. It can always be changed further.

@DarkLight1337
Copy link
Member Author

Yeah, perhaps it would be clearer as we make further changes down the line.

@simon-mo can you stamp this?

@DarkLight1337 DarkLight1337 merged commit aa39a8e into vllm-project:main Dec 5, 2024
71 checks passed
@DarkLight1337 DarkLight1337 deleted the usage-docs branch December 5, 2024 03:19
sleepwalker2017 pushed a commit to sleepwalker2017/vllm that referenced this pull request Dec 13, 2024
BKitor pushed a commit to BKitor/vllm that referenced this pull request Dec 30, 2024
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 ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants