Skip to content

Mllama(single + dual) + InternVL(single) + Llava (single) #267

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 28 commits into from
Feb 14, 2025
Merged

Mllama(single + dual) + InternVL(single) + Llava (single) #267

merged 28 commits into from
Feb 14, 2025

Conversation

ochougul
Copy link
Contributor

@ochougul ochougul commented Feb 10, 2025 •
edited
Loading

Adding generalized infrastructure to support VLMs with Dual/single QPC approaches

quic-rishinr and others added 15 commits February 10, 2025 21:09
@quic-rishinr @quic-amitraj
Mllama Vision support (#254)
48d24da 
Signed-off-by: Amit Raj <[email protected]>
Signed-off-by: Rishin Raj <[email protected]>
Co-authored-by: Amit Raj <[email protected]>
Signed-off-by: Amit Raj <[email protected]>
@quic-rishinr @quic-amitraj
Compiler command fix
0bdeea5 
Signed-off-by: Rishin Raj <[email protected]>
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
Mllama single qpc support added (#258)
c948607 
1. Mllama single qpc support added
2. Simplified generate inputs for single and dual qpc

---------

Signed-off-by: Amit Raj <[email protected]>
Co-authored-by: asmigosw <[email protected]>
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
Export fix
649cd32 
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
Generate fix-1
32f544c 
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
minor-fix
7ebf06b 
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
Model swap fix at the time of export and compile
3bc06be 
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
two_qpc_working
5accf3f 
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
Minor-fix-1
6ae6835 
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
working single and double with single soc
1e181f4 
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
ruff checks and format
5fb0acb 
Signed-off-by: Amit Raj <[email protected]>
@quic-rishinr @quic-amitraj
Updated factory class
87e07d0 
Signed-off-by: Rishin Raj <[email protected]>
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
Added support for Llava model single QPC (#265)
fc323f4 
Added support for Laava model single QPC

Signed-off-by: Amit Raj <[email protected]>
@ochougul @quic-amitraj
Added support for InternVL single QPC (#264)
1ac6b62 
Signed-off-by: Onkar Chougule <[email protected]>
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
final revision VLM
b3a5d22 
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj quic-amitraj force-pushed the mllama_single_dual_qpc branch from b9c0bc1 to b3a5d22 Compare February 10, 2025 21:10
@quic-amitraj
fixed liscence
d33e9a5 
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj
Minor fixes-1
905b703 
Signed-off-by: Amit Raj <[email protected]>
anujgupt-github
anujgupt-github reviewed Feb 11, 2025
View reviewed changes
QEfficient/transformers/models/modeling_auto.py

class QEFFTransformersBase(QEFFBaseModel):
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this entire part showing up as diff? possible to clean up the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because we moved the QEFFAutoModelForCausalLM class to the bottom of the file from top as we wanted to support Intern via that.

As we discussed should I chnage it to use QEFFAutoModelImageTextTOText?

vbaddi
vbaddi requested changes Feb 12, 2025
View reviewed changes
@ochougul
added warnings and auto_correct_inputs function
d1981c2 
Signed-off-by: Onkar Chougule <[email protected]>
@ochougul
remove unused imports
a32007e 
Signed-off-by: Onkar Chougule <[email protected]>
@ochougul
addressed comments
81cea10 
Signed-off-by: Onkar Chougule <[email protected]>
@quic-amitraj quic-amitraj force-pushed the mllama_single_dual_qpc branch from 24fad68 to 81cea10 Compare February 13, 2025 09:09
quic-hemagnih
quic-hemagnih reviewed Feb 13, 2025
View reviewed changes
Copy link
Contributor

@quic-hemagnih quic-hemagnih left a comment

Choose a reason for hiding this comment

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

Can we please add the description about newly added argument model

@ochougul ochougul mentioned this pull request Feb 13, 2025
Closed
@quic-amitraj quic-amitraj force-pushed the mllama_single_dual_qpc branch from 2de6982 to f5efdae Compare February 13, 2025 09:50
@quic-amitraj quic-amitraj mentioned this pull request Feb 13, 2025
Closed
@quic-amitraj
Addressed comments
ad594d7 
Signed-off-by: Amit Raj <[email protected]>
@quic-amitraj quic-amitraj force-pushed the mllama_single_dual_qpc branch from f5efdae to ad594d7 Compare February 13, 2025 11:49
quic-hemagnih
quic-hemagnih reviewed Feb 14, 2025
View reviewed changes
QEfficient/transformers/models/modeling_auto.py
config = AutoConfig.from_pretrained(pretrained_model_name_or_path, trust_remote_code=True)
config._attn_implementation = "eager"
config.vision_config.use_flash_attn = "false"
model = cls._hf_auto_class.from_pretrained(pretrained_model_name_or_path, config, *args, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we are explicitly fetching the config and directly calling the from_pretrained() of AutoModelForImageTextToText. We could have called super.from_pretrained() and then internally it would have handled this

quic-hemagnih
quic-hemagnih reviewed Feb 14, 2025
View reviewed changes
QEfficient/transformers/models/modeling_auto.py
logger.warning("Updating low_cpu_mem_usage=False")

kwargs.update({"attn_implementation": "eager", "low_cpu_mem_usage": False})
model = cls._hf_auto_class.from_pretrained(pretrained_model_name_or_path, *args, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of making these changes in derived class from_pretained() function shouldn't we make these changes in Base class and then call it from here, it might be good from design perspective

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We think we can remove QEFFTransformerBaseClass we will need a bigger discussion to decide this.

quic-hemagnih
quic-hemagnih reviewed Feb 14, 2025
View reviewed changes
QEfficient/transformers/models/modeling_auto.py
pretrained_model_name_or_path,
*args,
**kwargs,
):
if kwargs.get("attn_implementation", None) not in {None, "eager"}:
Copy link
Contributor

Choose a reason for hiding this comment

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

why we are not calling super.from_pretained() here?, it already has this peice of code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because we need to change config options in this method that are not needed for other auto clases.

@ochougul
minor bugfix
94e813c 
Signed-off-by: Onkar Chougule <[email protected]>
@ochougul
addressed comments
a661840 
Signed-off-by: Onkar Chougule <[email protected]>
@ochougul
removed image_text models tests to avoid pytest issues
ed7d5f2 
Signed-off-by: Onkar Chougule <[email protected]>
@ochougul ochougul merged commit d0ee7bc into main Feb 14, 2025
4 checks passed
This was referenced Feb 14, 2025
Closed
Closed
Closed
@vbaddi vbaddi mentioned this pull request Feb 14, 2025
Closed
quic-hemagnih pushed a commit to quic-hemagnih/efficient-transformers that referenced this pull request Mar 12, 2025
@ochougul @quic-rishinr
@quic-amitraj @quic-hemagnih
Mllama(single + dual) + InternVL(single) + Llava (single) (quic#267)
963986b 
Adding generalized infrastructure to support VLMs with Dual/single QPC
approaches

---------

Signed-off-by: Amit Raj <[email protected]>
Signed-off-by: Rishin Raj <[email protected]>
Signed-off-by: Onkar Chougule <[email protected]>
Co-authored-by: Rishin Raj <[email protected]>
Co-authored-by: Amit Raj <[email protected]>
Co-authored-by: Amit Raj <[email protected]>
Co-authored-by: asmigosw <[email protected]>
Signed-off-by: Hem Agnihotri <[email protected]>
@quic-rishinr quic-rishinr deleted the mllama_single_dual_qpc branch March 25, 2025 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anujgupt-github anujgupt-github anujgupt-github left review comments

@quic-amitraj quic-amitraj quic-amitraj left review comments

@quic-hemagnih quic-hemagnih quic-hemagnih left review comments

@vbaddi vbaddi vbaddi requested changes

@quic-rishinr quic-rishinr Awaiting requested review from quic-rishinr quic-rishinr is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ochougul @anujgupt-github @vbaddi @quic-amitraj @quic-hemagnih @quic-rishinr