Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

[Misc] Remove unused vllm/block.py #11336

Merged
merged 2 commits into from
Dec 19, 2024
Merged

[Misc] Remove unused vllm/block.py #11336

merged 2 commits into from
Dec 19, 2024

Conversation

Ghjk94522
Copy link
Contributor

@Ghjk94522 Ghjk94522 commented Dec 19, 2024 •
edited by github-actions bot
Loading

Since this file is no longer referenced, it can be removed from the project.

@github-actions GitHub Actions
Copy link

👋 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.

🚀

@Ghjk94522
* [Chore]
4a5184b 
description: remove the block py
solution: -
testcase: -

Signed-off-by: zhibin.yang <2500223255@qq.com>
@Ghjk94522
Copy link
Contributor Author

Ghjk94522 commented Dec 19, 2024 •
edited
Loading

@youkaichao @WoosukKwon
hope you can review this PR at your convenience.

@hmellor
Copy link
Member

hmellor commented Dec 19, 2024 •
edited
Loading

To add some additional context, the teo classes defined in this file:

  • PhysicalTokenBlock only used in vllm/block.py (the file being removed) and only referenced in comments in vllm/core/evictor.py
  • BlockTable also defined in vllm/core/block/block_table.py and is only every imported from the latter file

@mgoin mgoin changed the title * [Misc] remove the block.py in vllm [Misc] Remove unused vllm/block.py Dec 19, 2024
@mgoin
Copy link
Member

mgoin commented Dec 19, 2024 •
edited
Loading

LGTM, I also don't see these classes used in code anywhere.

PhysicalTokenBlock only used in vllm/block.py (the file being removed) and only referenced in comments in vllm/core/evictor.py

The comments in vllm/core/evictor.py should be updated to reflect the removal of this class.

@hmellor
Copy link
Member

hmellor commented Dec 19, 2024

You're right, the new class to reference is Block which is defined in vllm/core/block/interfaces.py

@comaniac comaniac added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 19, 2024
@comaniac comaniac enabled auto-merge (squash) December 19, 2024 16:00
@comaniac comaniac merged commit e461c26 into vllm-project:main Dec 19, 2024
64 checks passed
@Ghjk94522 Ghjk94522 deleted the remove_block_1219 branch December 20, 2024 01:07
lucas-tucker pushed a commit to lucas-tucker/vllm-lucas-tucker that referenced this pull request Dec 21, 2024
@Ghjk94522
[Misc] Remove unused vllm/block.py (vllm-project#11336)
41bd782 
Signed-off-by: lucast2021 <lucast2021@headroyce.org>
BKitor pushed a commit to BKitor/vllm that referenced this pull request Dec 30, 2024
@Ghjk94522 @BKitor
[Misc] Remove unused vllm/block.py (vllm-project#11336)
9230b2f
joennlae pushed a commit to 44ai-labs/vllm that referenced this pull request Jan 19, 2025
@Ghjk94522
[Misc] Remove unused vllm/block.py (vllm-project#11336)
e3d9909
joennlae pushed a commit to 44ai-labs/vllm that referenced this pull request Jan 19, 2025
@Ghjk94522
[Misc] Remove unused vllm/block.py (vllm-project#11336)
bb08239
abmfy pushed a commit to abmfy/vllm-flashinfer that referenced this pull request Jan 24, 2025
@Ghjk94522 @abmfy
[Misc] Remove unused vllm/block.py (vllm-project#11336)
3f2fe41 
Signed-off-by: Bowen Wang <abmfy@icloud.com>
abmfy pushed a commit to abmfy/vllm-flashinfer that referenced this pull request Jan 24, 2025
@Ghjk94522 @abmfy
[Misc] Remove unused vllm/block.py (vllm-project#11336)
8f7d1c1
mzusman pushed a commit to mzusman/vllm that referenced this pull request Mar 12, 2025
@Ghjk94522 @mzusman
[Misc] Remove unused vllm/block.py (vllm-project#11336)
1362978
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@comaniac comaniac comaniac approved these changes

@zhuohan123 zhuohan123 Awaiting requested review from zhuohan123 zhuohan123 is a code owner

@youkaichao youkaichao Awaiting requested review from youkaichao youkaichao is a code owner

@alexm-redhat alexm-redhat Awaiting requested review from alexm-redhat alexm-redhat is a code owner

@njhill njhill Awaiting requested review from njhill njhill is a code owner

Assignees
No one assigned
Labels
ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Ghjk94522 @hmellor @mgoin @comaniac