-
Notifications
You must be signed in to change notification settings - Fork 44
[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
Conversation
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>
QEfficient/utils/_utils.py
Outdated
@@ -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 |
There was a problem hiding this comment.
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)
Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
LGTM |
Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
QEfficient/utils/_utils.py
Outdated
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 |
There was a problem hiding this comment.
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>
Thanks @abukhoy for incorporating the comment. LGTM |
Signed-off-by: Abukhoyer Shaik <quic_abukhoye@quicinc.com>
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>
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>
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.