[Model]Refactor MiniCPMV

[jeejeelee]

I have completed the following modification:

• Separate different versions of MiniCPMV to facilitate support for future features like LoRA and BNB.
• Port Idefics2VisionTransformer

ping @ywang96 @DarkLight1337 @HwwwwwwwH

[github-actions]

👋 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 consists a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of default ones by unblocking the steps in your fast-check build on Buildkite UI.

Once the PR is approved and ready to go, please make sure to run full CI as it is required to merge (or just use auto-merge).

To run full CI, you can do one of these:

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

🚀

[ywang96]

Hey @jeejeelee Thanks for the contribution! Were you able to verify if the model works on TP=1,2,4,8? (I will verify this myself later, but I was curious if this PR is ready for testing)

Also I'm curious if you have seen any significant speedup by sharding the ViT

[jeejeelee]

Hey @jeejeelee Thanks for the contribution! Were you able to verify if the model works on TP=1,2,4,8? (I will verify this myself later, but I was curious if this PR is ready for testing)

Also I'm curious if you have seen any significant speedup by sharding the ViT

Just verified on TP=1,2,4 . Currently, I do not have available 8-gpu resources and have not yet verified TP=8

[DarkLight1337]

This will cause significant merge conflicts with #6995. Would it be better for you to incorporate my changes into your PR instead of releasing them separately?

[ywang96]

This will cause significant merge conflicts with #6995. Would it be better for you to incorporate my changes into your PR instead of releasing them separately?

I agree with this too.

[jeejeelee]

This will cause significant merge conflicts with #6995. Would it be better for you to incorporate my changes into your PR instead of releasing them separately?

Ok, I will incorporate your changes ASAP

[jeejeelee]

Also I'm curious if you have seen any significant speedup by sharding the ViT

@ywang96 I have not yet tested the speedup effect of TP, but I will provide test results once available.

[jeejeelee]

@DarkLight1337 It seems #6995 has a bug. After incorporating your changes, the generated results have become poor.

[DarkLight1337]

Try reverting the lines where get_2d_sincos_pos_embed is called.

[jeejeelee]

Try reverting the lines where get_2d_sincos_pos_embed is called.

I have figured it out, this snippet has a bug, and have fixed it

[DarkLight1337]

Try reverting the lines where get_2d_sincos_pos_embed is called.

I have figured it out, this snippet has a bug, and have fixed it

My bad, I forgot to rename the intermediate variables. Thanks for fixing this!

To avoid confusion, I have closed the other PR.

[jeejeelee]

Try reverting the lines where get_2d_sincos_pos_embed is called.

I have figured it out, this snippet has a bug, and have fixed it

My bad, I forgot to rename the intermediate variables. Thanks for fixing this!

To avoid confusion, I have closed the other PR.

Thanks, could you help review my implementation? I want to complete this PR asap. My final goal is actually to make minicpmv2.5 support LoRA

[HwwwwwwwH]

Sry for late. Really appreciate your contribution! I'll check these modifications.

[HwwwwwwwH]

I think it's truly great to have MiniCPMV separated. I'm pulling the code of this PR and running some evaluations.

[jeejeelee]

@DarkLight1337 @HwwwwwwwH It's getting late here, so I'll log off for now. Thank you for all your hard work

[DarkLight1337]

@DarkLight1337 @HwwwwwwwH It's getting late here, so I'll log off for now. Thank you for all your hard work

Sure, sorry for interfering with your own testing...

[DarkLight1337]

@DarkLight1337 After updating the latest changes, I'm still encountering errors

[rank0]:   File "/mypath/vllm/vllm/model_executor/models/minicpmv.py", line 588, in _parse_and_validate_inputs
[rank0]:     raise ValueError(f"Inconsistent flattened lengths, found: {lens}")
[rank0]: ValueError: Inconsistent flattened lengths, found: [0, 16, 16]

After some offline discussion with @HwwwwwwwH , apparently the dummy data doesn't contain image tokens while providing the image. I have updated the validation to allow this for now, we will revisit the dummy data generation in a later PR.

[HwwwwwwwH]

te here, so I'll log off for now. Thank you for all your hard work

Thank you too. Good night!

[HwwwwwwwH]

@jeejeelee Hi there. Could you run the model correctly now? I got some problems.

[jeejeelee]

@jeejeelee Hi there. Could you run the model correctly now? I got some problems.

Me too.

[HwwwwwwwH]

@DarkLight1337 Thank you for your hard work. It works fine for me now.

[ywang96]

Hello @jeejeelee! Just a follow-up question: are you interested in implementing Idefics3 eventually? (It's not available on transformers yet since PR for this model is still WIP)

[jeejeelee]

Hello @jeejeelee! Just a follow-up question: are you interested in implementing Idefics3 eventually? (It's not available on transformers yet since PR for this model is still WIP)

I'd be happy to implement this, but I might not be able to start working on it until next week.

[ywang96]

Hello @jeejeelee! Just a follow-up question: are you interested in implementing Idefics3 eventually? (It's not available on transformers yet since PR for this model is still WIP)

I'd be happy to implement this, but I might not be able to start working on it until next week.

@jeejeelee No rush at all and thank you for the interest. Most likely we'll have to wait for the transformers PR to get into a better shape so we can verify model correctness anyways.