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

[Fix] Address remaining issues of supporting MiniCPMV #2977

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

mickqian
Copy link
Contributor

Motivation

Address remaining issues of #2785

Modifications

  1. Update document of implementing a new vision-llm
  2. Add some test for comparing logits output of SGLang and HF
  3. Code cleanup

Checklist

@mickqian mickqian mentioned this pull request Jan 19, 2025
3 tasks
@mickqian mickqian force-pushed the minicpmv branch 9 times, most recently from d4a7a2d to d32da67 Compare January 19, 2025 17:04
@zhaochenyang20
Copy link
Collaborator

Great work! Once ready, please ask us to review.

@merrymercy
Copy link
Contributor

also remove these vllm dependency

from vllm.distributed import parallel_state
from vllm.distributed import utils as dist_utils

@mickqian mickqian force-pushed the minicpmv branch 9 times, most recently from 6f78efe to 797ac44 Compare January 21, 2025 14:35
@mickqian mickqian marked this pull request as ready for review January 22, 2025 03:56
@zhaochenyang20
Copy link
Collaborator

Cool. I will ask @yizhang2077 to help~

"tgt_sizes": [inputs["tgt_sizes"]],
"im_start_id": [self.tokenizer.im_start_id],
"im_end_id": [self.tokenizer.im_end_id],
"slice_start_id": [self.tokenizer.slice_start_id],
Copy link
Collaborator

@yizhang2077 yizhang2077 Jan 23, 2025

Choose a reason for hiding this comment

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

Cool! I think maybe this test can be more general so that it can also be used by qwen2vl or more VLM?

@zhaochenyang20
Copy link
Collaborator

@mickqian mick, great work. So we are ended and just need to past the review? Thanks!

@mickqian mickqian force-pushed the minicpmv branch 7 times, most recently from b09c28b to bee8340 Compare January 25, 2025 04:07
@mickqian mickqian changed the title [Fix] Address remain issues of supporting MiniCPMV [Fix] Address remaining issues of supporting MiniCPMV Jan 25, 2025
@yizhang2077
Copy link
Collaborator

@mickqian there are still some failed cases, but it seems they are not related to your pr?

@zhaochenyang20
Copy link
Collaborator

@mickqian there are still some failed cases, but it seems they are not related to your pr?

Yeah. CI is wrong possibly due to kernel or largely refactor.

@yizhang2077
Copy link
Collaborator

Yeah. CI is wrong possibly due to kernel or largely refactor.

Since test_vision_*.py has been all passed, I think we can approve this pr

@zhaochenyang20 zhaochenyang20 merged commit 9f635ea into sgl-project:main Jan 28, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants