Skip to content

QNN Compilation path Support in QEFFBaseModel class. #335

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 3 commits into from
Apr 21, 2025
Merged

QNN Compilation path Support in QEFFBaseModel class. #335

merged 3 commits into from
Apr 21, 2025

Conversation

shubhagr-quic
Copy link
Contributor

This change will facilitate the support of QNN Compilation path for any model class derived from QEFFBaseModel class.

@shubhagr-quic shubhagr-quic force-pushed the qnnCompilationQEFFBaseModel branch from 4f60ecd to a84bec5 Compare April 1, 2025 11:06
@shubhagr-quic shubhagr-quic marked this pull request as draft April 1, 2025 11:08
@shubhagr-quic shubhagr-quic force-pushed the qnnCompilationQEFFBaseModel branch 2 times, most recently from 5bcedfe to 4a4519b Compare April 3, 2025 11:36
@shubhagr-quic shubhagr-quic marked this pull request as ready for review April 3, 2025 11:37
@shubhagr-quic shubhagr-quic force-pushed the qnnCompilationQEFFBaseModel branch from 4a4519b to 4ba2c26 Compare April 3, 2025 11:46
@shubhagr-quic shubhagr-quic force-pushed the qnnCompilationQEFFBaseModel branch 2 times, most recently from de8dbc3 to 2f352b6 Compare April 4, 2025 18:59
@shubhagr-quic shubhagr-quic force-pushed the qnnCompilationQEFFBaseModel branch 3 times, most recently from 739d571 to ee2faf1 Compare April 9, 2025 10:11
@quic-amitraj quic-amitraj marked this pull request as draft April 11, 2025 12:19
@shubhagr-quic shubhagr-quic force-pushed the qnnCompilationQEFFBaseModel branch from ac90ef3 to 619a718 Compare April 21, 2025 05:30
@abukhoy
[Docs Fix: ]QEFFAutoModelForImageTextToText Typing mistake Resolved (q…
bdcd7e5 
…uic#357)

This PR is created for resolving a typing mistake in the name of
`QEFFAutoModelForImageTextToText ` in the `validate.md`

Signed-off-by: Abukhoyer Shaik <[email protected]>
@shubhagr-quic shubhagr-quic force-pushed the qnnCompilationQEFFBaseModel branch 3 times, most recently from 6c67b3c to c3fe1bb Compare April 21, 2025 06:23
@shubhagr-quic
QNN Compilation path Support in QEFFBaseModel class.
540232a 
Signed-off-by: Shubham Agrawal <[email protected]>

Added enable_qnn as arg in _compile

Signed-off-by: Shubham Agrawal <[email protected]>

Reduced number of models tested for QNN, and skipped Whisper model

Signed-off-by: Shubham Agrawal <[email protected]>
@shubhagr-quic shubhagr-quic force-pushed the qnnCompilationQEFFBaseModel branch from c3fe1bb to 540232a Compare April 21, 2025 06:26
shubhagr-quic
shubhagr-quic commented Apr 21, 2025
View reviewed changes
tests/transformers/spd/test_pld_inference.py Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed because of Format failure

shubhagr-quic
shubhagr-quic commented Apr 21, 2025
View reviewed changes
tests/transformers/spd/test_spd_inference.py Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed because of Format failure

@shubhagr-quic shubhagr-quic marked this pull request as ready for review April 21, 2025 08:01
quic-amitraj
quic-amitraj requested changes Apr 21, 2025
View reviewed changes
QEfficient/base/modeling_qeff.py
"""
if enable_qnn:
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to have two different python file qnn_compiler.py and another qaic_compiler.py, which will have all the code of compilation on both the different sdks. _compile method will route to the compilation based on passed parameters.@quic-rishinr

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree. Lets take it as a separate change post merging this 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.

@quic-amitraj
Will take up the change to move _qnn_compile functionality to qnn_compile as a separate change because of the following reason:
qnn_compile function in qnn_compiler.py is also used by CLI commands for compilation which dumps qpcs into folder with name qpc_base_path/qpcs and qpcs compiled via QEFFBaseModel class should be dumped in qpc folder with config hash. Moving config hash creation in qnn_compile function will force CLI commands compilation as well to follow same directory structure and will fail CLI unit testing where the qpc folder to check is hard coded as "qpcs".

QEfficient/base/modeling_qeff.py
@@ -346,35 +370,27 @@ def _qnn_compile(
onnx_path: Optional[str] = None,
compile_dir: Optional[str] = None,
*,
custom_io: Optional[Dict[str, str]] = None,
Copy link
Contributor

@quic-amitraj quic-amitraj Apr 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

After addressing then above comment there will be no need of this method to be here. All the preprocessing to compilation should be done inside the their specific file. This way we will have clean code and easier to understand.

scripts/Jenkinsfile
@@ -117,7 +117,7 @@ pipeline {
}
stage('QNN Non-CLI Tests') {
steps {
timeout(time: 60, unit: 'MINUTES') {
timeout(time: 200, unit: 'MINUTES') {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the expected time to pass this stage after this changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

timeout is set to 200 minutes in order to align with the similar stage of QAIC.
I have not been able to get a CI run completed after reducing the models to be tested on QNN. Should not take more than 30 mins.

@shubhagr-quic
Incorporated Comments
89ab6fa 
	1. Changed decode_only to prefill_only
	2. Added comments for compiler_options in derived class compile API
	3. Renamed test_models to test_models_qaic

Signed-off-by: Shubham Agrawal <[email protected]>
@quic-rishinr quic-rishinr changed the base branch from main to release/v1.20.0 April 21, 2025 16:11
@quic-rishinr quic-rishinr merged commit 8a55a74 into quic:release/v1.20.0 Apr 21, 2025
5 checks passed
@quic-rishinr quic-rishinr deleted the qnnCompilationQEFFBaseModel branch April 21, 2025 17:55
shubhagr-quic added a commit to shubhagr-quic/efficient-transformers that referenced this pull request Apr 22, 2025
@shubhagr-quic
QNN Compilation path Support in QEFFBaseModel class. (quic#335)
b01f93f 
This change will facilitate the support of QNN Compilation path for any
model class derived from QEFFBaseModel class.

---------
Signed-off-by: Shubham Agrawal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@quic-rishinr quic-rishinr quic-rishinr approved these changes

@quic-hemagnih quic-hemagnih quic-hemagnih approved these changes

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

@quic-amitraj quic-amitraj Awaiting requested review from quic-amitraj

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

Successfully merging this pull request may close these issues.
5 participants
@shubhagr-quic @quic-amitraj @quic-rishinr @quic-hemagnih @abukhoy