Skip to content

[Model] use AutoWeightsLoader for bart #18299

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

Merged
merged 3 commits into from
Jul 20, 2025

Conversation

calvin0327
Copy link
Contributor

@calvin0327 calvin0327 commented May 17, 2025

FIX (partial) #15697

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.

🚀

@calvin0327
Copy link
Contributor Author

@lengrongfu PTAL

@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch from e647f4b to af0f5d3 Compare May 17, 2025 08:24
@lengrongfu
Copy link
Contributor

Thanks for working on this, there are some comments.

@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch from af0f5d3 to 5243921 Compare May 17, 2025 09:53
@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch from 5243921 to 1a1b005 Compare May 18, 2025 03:24
@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch from 1a1b005 to d2a3759 Compare May 18, 2025 04:02
@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch 2 times, most recently from 74530cb to 340f7e3 Compare May 20, 2025 06:44
@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch 2 times, most recently from 45f885f to 81f8de8 Compare June 1, 2025 05:13
@calvin0327
Copy link
Contributor Author

test:
image

@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch from 81f8de8 to dfc3af8 Compare June 1, 2025 05:24
@calvin0327
Copy link
Contributor Author

@DarkLight1337 @noooop I've resolved it, and BART can now be successfully loaded.

@noooop
Copy link
Contributor

noooop commented Jun 1, 2025

Is BartModel.load_weight not used and can be deleted? Because BartForConditionalGeneration already uses AutoWeightsLoader.

or

Can the BartModel.load_weights logic be simplified using AutoWeightsLoader?

then

BartForConditionalGeneration.load_weights:

    def load_weights(self, weights: Iterable[tuple[str, torch.Tensor]]):
        return self.model.load_weights(weights, mapper=self.hf_to_vllm_mapper)

The final result of AutoWeightsLoader replaces each model's load_weights function.

Now the load_weights logic still not simplified.

@calvin0327
Copy link
Contributor Author

Is BartModel.load_weight not used and can be deleted? Because BartForConditionalGeneration already uses AutoWeightsLoader.

or

Can the BartModel.load_weights logic be simplified using AutoWeightsLoader?

then

BartForConditionalGeneration.load_weights:

    def load_weights(self, weights: Iterable[tuple[str, torch.Tensor]]):
        return self.model.load_weights(weights, mapper=self.hf_to_vllm_mapper)

The final result of AutoWeightsLoader replaces each model's load_weights function.

Now the load_weights logic still not simplified.

@noooop Now,BartModel.load_weights is used, it be called by BartForConditionalGeneration.load_weights. I've noticed that other models also add the load_weights method to their base models, which is then invoked via AutoWeightsLoader.

Please correct me if I'm misunderstanding.

@noooop
Copy link
Contributor

noooop commented Jun 3, 2025

You have optimized the logic related to lm_head and tie_word_embeddings.

Try using AutoWeightsLoader to simplify the BartModel.load_weights logic.

@calvin0327
Copy link
Contributor Author

@noooop Thank you for your reply, but I haven't found a way to optimize the BartModel.load_weight, could you give me some hints?

@noooop
Copy link
Contributor

noooop commented Jul 14, 2025

Please refer to #20534

@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch 5 times, most recently from a49df83 to eeb7349 Compare July 16, 2025 05:43
@calvin0327
Copy link
Contributor Author

This is test result:
image

image

@calvin0327
Copy link
Contributor Author

@noooop Thanks for your hints, I had optimized the load_weights for BartModel, PTAL

@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch from eeb7349 to 35cdd55 Compare July 16, 2025 05:54
@noooop
Copy link
Contributor

noooop commented Jul 16, 2025

@noooop Thanks for your hints, I had optimized the load_weights for BartModel, PTAL

Please see the message above.

@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch 4 times, most recently from 3e97f9e to 06e4c17 Compare July 16, 2025 09:35
@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch 2 times, most recently from 1656f62 to b244c9c Compare July 17, 2025 08:10
Copy link
Contributor

@noooop noooop left a comment

Choose a reason for hiding this comment

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

thanks for your contribution.

cc @DarkLight1337

@DarkLight1337 DarkLight1337 enabled auto-merge (squash) July 17, 2025 10:03
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Jul 17, 2025
auto-merge was automatically disabled July 18, 2025 02:06

Head branch was pushed to by a user without write access

@calvin0327 calvin0327 force-pushed the autoweights-for-bart branch from b244c9c to e22db99 Compare July 18, 2025 02:06
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) July 18, 2025 03:11
@noooop
Copy link
Contributor

noooop commented Jul 18, 2025

v1/entrypoints/openai/test_multi_api_servers.py::test_single_completion[ibm-research/PowerMoE-3b] Testing locally

I think it's failure not related to this PR's , but it has failed twice already.


can pass locally, I don't know why it failed.

@noooop
Copy link
Contributor

noooop commented Jul 18, 2025

@calvin0327

I found another failure related to ibm-research/PowerMoE-3b, so it's failure not related to this PR's

https://buildkite.com/vllm/ci/builds/24219/steps/canvas?sid=019818b9-c1bf-4856-909d-04ed3afc8696

Try tomorrow, click on calvin0327:autoweights-for-bart under the title -> sync fork -> update branch to merge into main and trigger CI. Also, auto-merge will not be canceled. Do not submit new code, it will cancel auto-merge.

@calvin0327
Copy link
Contributor Author

@calvin0327

I found another failure related to ibm-research/PowerMoE-3b, so it's failure not related to this PR's

https://buildkite.com/vllm/ci/builds/24219/steps/canvas?sid=019818b9-c1bf-4856-909d-04ed3afc8696

Try tomorrow, click on calvin0327:autoweights-for-bart under the title -> sync fork -> update branch to merge into main and trigger CI. Also, auto-merge will not be canceled. Do not submit new code, it will cancel auto-merge.

Ok. thanks.

@DarkLight1337 DarkLight1337 merged commit 51ba839 into vllm-project:main Jul 20, 2025
69 checks passed
LyrisZhong pushed a commit to LyrisZhong/vllm that referenced this pull request Jul 23, 2025
avigny pushed a commit to avigny/vllm that referenced this pull request Jul 31, 2025
x22x22 pushed a commit to x22x22/vllm that referenced this pull request Aug 5, 2025
Pradyun92 pushed a commit to Pradyun92/vllm that referenced this pull request Aug 6, 2025
npanpaliya pushed a commit to odh-on-pz/vllm-upstream that referenced this pull request Aug 6, 2025
jinzhen-lin pushed a commit to jinzhen-lin/vllm that referenced this pull request Aug 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants