Skip to content

[Bug Fix]: qpc sdk config dump issue fixing #379

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 14 commits into from
May 8, 2025
Merged

Conversation

abukhoy
Copy link
Contributor

@abukhoy abukhoy commented Apr 23, 2025

This PR addresses an issue causing the qconfig dump to fail when QAIC is not installed but QNN is present. With this update, the qconfig will be successfully dumped in both QAIC and QNN environments.

abukhoy added 6 commits March 6, 2025 15:35
Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
@@ -492,7 +492,7 @@ def create_and_dump_qconfigs(
many other compilation options.
"""
qnn_config = compiler_options["qnn_config"] if "qnn_config" in compiler_options else None
Copy link
Contributor

Choose a reason for hiding this comment

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

you can replace it as compiler_options.get("qnn_config", None)

@quic-rishinr quic-rishinr marked this pull request as draft April 25, 2025 04:04
@asmigosw
Copy link
Contributor

asmigosw commented May 5, 2025

LGTM

@abukhoy abukhoy marked this pull request as ready for review May 5, 2025 10:32
@abukhoy abukhoy requested a review from quic-amitraj as a code owner May 5, 2025 10:32
@abukhoy abukhoy marked this pull request as draft May 6, 2025 11:48
abukhoy added 2 commits May 7, 2025 14:26
Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
@abukhoy abukhoy marked this pull request as ready for review May 7, 2025 10:13
qnn_sdk_yaml_path = os.path.join(qnn_sdk_path, QnnConstants.QNN_SDK_YAML)
with open(qnn_sdk_yaml_path, "r") as file:
qnn_sdk_details = yaml.safe_load(file)
qnn_config_path = qnn_config if qnn_config is not None else QnnConstants.QNN_DEFAULT_CONFIG_PATH
Copy link
Contributor

Choose a reason for hiding this comment

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

If qnn_config is passed as None, we should be mentioning qnn_config_path as None only, since QNN_DEFAULT_CONFIG_PATH will not be used here. Also, in case where we are using qefficient as a package and dont have access to the qefficient repository, qnn_config will not be present in the default path.

@quic-rishinr Please validate the observation.

Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
@shubhagr-quic
Copy link
Contributor

Thanks @abukhoy for incorporating the comment. LGTM

Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
@quic-rishinr quic-rishinr merged commit 4e5c95a into quic:main May 8, 2025
4 of 5 checks passed
mohiso22 pushed a commit to mohiso22/efficient_transformers that referenced this pull request May 14, 2025
This PR addresses an issue causing the qconfig dump to fail when QAIC is
not installed but QNN is present. With this update, the qconfig will be
successfully dumped in both QAIC and QNN environments.

---------

Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
Signed-off-by: Mohit Soni <quic_mohisoni@quicinc.com>
mohiso22 pushed a commit to mohiso22/efficient_transformers that referenced this pull request May 14, 2025
This PR addresses an issue causing the qconfig dump to fail when QAIC is
not installed but QNN is present. With this update, the qconfig will be
successfully dumped in both QAIC and QNN environments.

---------

Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
Signed-off-by: Mohit Soni <quic_mohisoni@quicinc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants