Qualcomm AI Engine Direct - Test Framework Refactor#19660
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19660
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New Failures, 4 Unrelated Failures, 3 Unclassified FailuresAs of commit 7f46b92 with merge base e28ef13 ( NEW FAILURES - The following jobs have failed:
UNCLASSIFIED FAILURES - DrCI could not classify the following jobs because the workflow did not run on the merge base. The failures may be pre-existing on trunk or introduced by this PR:
FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
29b0b6b to
bdc8e4c
Compare
|
|
|
|
Hi @psiddh, thanks for the comments.
name: Qualcomm
on:
pull_request:
paths:
- backends/qualcomm/**
- examples/qualcomm/**
paths-ignore:
- '**.md'
jobs:
prepare:
runs-on: ubuntu-latest
outputs:
run_backend_e2e_oss: ${{ steps.parse.outputs.run_backend_e2e_oss }}
run_backend_e2e_genai: ${{ steps.parse.outputs.run_backend_e2e_genai }}
run_backend_feature: ${{ steps.parse.outputs.run_backend_feature }}
run_backend_op: ${{ steps.parse.outputs.run_backend_op }}
run_common_pass: ${{ steps.parse.outputs.run_common_pass }}
run_common_utils: ${{ steps.parse.outputs.run_common_utils }}
steps:
- name: checkout commit
uses: actions/checkout@v3
- name: build commit
run: |
echo "build finished"
- name: parse labels
id: parse
shell: bash
run: |
set -eux
#files=$(git diff --name-only ${{ github.event.pull_request.base.ref }} HEAD | xargs)
files="utils"
if [[ $files =~ examples && ! $files =~ llama ]]; then
echo "run_backend_e2e_oss=1" >> $GITHUB_OUTPUT
fi
if [[ $files =~ llama ]]; then
echo "run_backend_e2e_genai=1" >> $GITHUB_OUTPUT
fi
if [[ $files =~ aot|runtime|serialization ]]; then
echo "run_backend_feature=1" >> $GITHUB_OUTPUT
fi
if [[ $files =~ aot|builders|quantizer|runtime|serialization ]]; then
echo "run_backend_op=1" >> $GITHUB_OUTPUT
fi
if [[ $files =~ _passes ]]; then
echo "run_common_pass=1" >> $GITHUB_OUTPUT
fi
if [[ $files =~ utils ]]; then
echo "run_common_utils=1" >> $GITHUB_OUTPUT
fi
# test-backend-e2e-oss
test-backend-e2e-oss-gpu:
runs-on: ubuntu-latest
needs: prepare
if: needs.prepare.outputs.run_backend_e2e_oss
steps:
- name: run test
run: |
echo "test finished"
test-backend-e2e-oss-htp:
runs-on: ubuntu-latest
needs: prepare
continue-on-error: true
strategy:
max-parallel: 6
matrix:
arch: [v68, v69, v73, v75, v79, v81]
if: needs.prepare.outputs.run_backend_e2e_oss
steps:
- name: run test
run: |
echo "test finished"
test-backend-e2e-oss-lpai:
runs-on: ubuntu-latest
needs: prepare
continue-on-error: true
strategy:
max-parallel: 1
matrix:
arch: [v6]
if: needs.prepare.outputs.run_backend_e2e_oss
steps:
- name: run test
run: |
echo "test finished"
# test-backend-e2e-genai
test-backend-e2e-genai-gpu:
runs-on: ubuntu-latest
needs: prepare
if: needs.prepare.outputs.run_backend_e2e_genai
steps:
- name: run test
run: |
echo "test finished"
test-backend-e2e-genai-htp:
runs-on: ubuntu-latest
needs: prepare
continue-on-error: true
strategy:
max-parallel: 6
matrix:
arch: [v68, v69, v73, v75, v79, v81]
if: needs.prepare.outputs.run_backend_e2e_genai
steps:
- name: run test
run: |
echo "test finished"
test-backend-e2e-genai-lpai:
runs-on: ubuntu-latest
needs: prepare
continue-on-error: true
strategy:
max-parallel: 1
matrix:
arch: [v6]
if: needs.prepare.outputs.run_backend_e2e_genai
steps:
- name: run test
run: |
echo "test finished"
# test-backend-feature
test-backend-feature-gpu:
runs-on: ubuntu-latest
needs: prepare
if: needs.prepare.outputs.run_backend_feature
steps:
- name: run test
run: |
echo "test finished"
test-backend-feature-htp:
runs-on: ubuntu-latest
needs: prepare
continue-on-error: true
strategy:
max-parallel: 6
matrix:
arch: [v68, v69, v73, v75, v79, v81]
if: needs.prepare.outputs.run_backend_feature
steps:
- name: run test
run: |
echo "test finished"
test-backend-feature-lpai:
runs-on: ubuntu-latest
needs: prepare
continue-on-error: true
strategy:
max-parallel: 1
matrix:
arch: [v6]
if: needs.prepare.outputs.run_backend_feature
steps:
- name: run test
run: |
echo "test finished"
# test-backend-operator
test-backend-operator-gpu:
runs-on: ubuntu-latest
needs: prepare
if: needs.prepare.outputs.run_backend_op
steps:
- name: run test
run: |
echo "test finished"
test-backend-operator-htp:
runs-on: ubuntu-latest
needs: prepare
continue-on-error: true
strategy:
max-parallel: 6
matrix:
arch: [v68, v69, v73, v75, v79, v81]
if: needs.prepare.outputs.run_backend_op
steps:
- name: run test
run: |
echo "test finished"
test-backend-operator-lpai:
runs-on: ubuntu-latest
needs: prepare
continue-on-error: true
strategy:
max-parallel: 1
matrix:
arch: [v6]
if: needs.prepare.outputs.run_backend_op
steps:
- name: run test
run: |
echo "test finished"
# test-backend-common-pass
test-backend-common-pass:
runs-on: ubuntu-latest
needs: prepare
if: needs.prepare.outputs.run_common_pass
steps:
- name: run test
run: |
echo "test finished"
# test-backend-common-utils
test-backend-common-utils:
runs-on: ubuntu-latest
needs: prepare
if: needs.prepare.outputs.run_common_utils
steps:
- name: run test
run: |
echo "test finished"
|
|
Requesting @mergennachin @digantdesai @JacobSzwejbka to also take a look at this PR |
|
@claude review this PR. |
|
Claude finished @digantdesai's task in 2m 58s —— View job Review of PR #19660: Qualcomm AI Engine Direct - Test Framework Refactor
Overall AssessmentThis is a well-structured refactoring that introduces a pytest-based test framework for Qualcomm backend testing, replacing/augmenting the existing unittest-based approach. The architecture is clean, with good separation of concerns between test infrastructure ( The PR also includes meaningful functional improvements: merging Architecture & Design (Positive)
Issues & SuggestionsHigh Priority1. Bare try:
dtype_info = torch.iinfo(
reduce(getattr, ["output_qspec", "dtype"], quant_info)
)
except:
returnThis silently swallows all exceptions including 2. Use of if all(
[
node.op == "call_function",
node.target in candidates,
quant_range := self._get_quant_range(node),
]
):Using if (node.op == "call_function"
and node.target in candidates
and (quant_range := self._get_quant_range(node))):3. mode = "emulator" if qnn_config.build_folder == "build-x86" else "remote"
globals()[f"verify_output_{mode}"](...)This is fragile and hard to grep for callers. A simple dict or if/else would be safer and more readable: verify_fn = verify_output_emulator if qnn_config.build_folder == "build-x86" else verify_output_remote
verify_fn(...)Medium Priority4. The 5. Test file uses star import ( from executorch.backends.qualcomm.tests.rework.src.op import * # noqa: F403While pragmatic (avoids listing 133 classes), this makes it impossible to determine which names are used without runtime introspection. Since each test function explicitly calls a specific class, consider whether a grouped import or 6. Commented-out test cases in existing test file ( # AvgPoolModule((1280, 1280), (1280, 1280), (0, 0), True), # noqa: F405
# AvgPoolModule((1280, 1280), (1280, 1280), (320, 320), True), # noqa: F405These should either be removed entirely (if the behavior is now handled by the new framework) or replaced with 7. The 8. Hardcoded device workspace path ( device_workspace = f"{getattr(qnn_config, 'device_workspace', '')}_{qnn_config.backend}"
Low Priority9. elapsed_time = time.time() - terminalreporter._session_start.timeRelying on private 10. Good addition. However, 11. Missing The Functional Changes (Non-test)The following functional changes look correct:
SummaryThis is a solid refactoring with good test infrastructure design. The main concerns are:
The architecture scales well for the follow-up PRs (GPU, LPAI, v69-v81 tests). |
digantdesai
left a comment
There was a problem hiding this comment.
Took a quick look. LGTM. Asked Claude review for nits. Thanks.
Please create issues, and a tracker issue for this |
|
@psiddh - @abhinaykukkadapu was trying to run these in OSS using BUCK for internal CI parity, are you planning to continue that? |
Yes @digantdesai I had a chat about this with @abhinaykukkadapu . Afaiu, BUCK'fying OSS is mostly done (80 - 90%). There are apparently few edge cases that may still break. The plan / next steps is to closely monitor any breakages due edge cases and fix them, as they surface. |
|
Lint / link-check / lint-urls (pull_request)Failing after 28s needs to be fixed before this can be merged |
|
@haowhsu-quic Can you rebase / resolve conflicts please ? Also fix lint checks, and we can land this PR |
Co-authored-by: @winskuo-quic, @chenweng-quic - introduce pytest and reorganize the file architecture for finer-grained testing - wider coverage of operator test with combinations of different precisions, codebase was changed accordingly - add feature tests for HTP
|
@claude Review the build failures on PR and check if they are related to the ongoing PR |
|
Claude finished @psiddh's task in 1m 58s —— View job Build failure analysis
Conclusion: none of the failures are related to this PR. This PR only touches Breakdown of the 4 failing workflow runs:
Details on the two that look the scariest: 1. The traceback originates in 2. Aarch64 wheel — CoreML: A missing standard-integer-header include in the CoreML in-memory filesystem. Dr. CI itself notes this job did not run on the merge base, so it cannot attribute it — but it is in The Qualcomm jobs Dr. CI listed under FLAKY ( Recommendation: none of these block the PR on its own merits. A rebase onto current |
Summary
Co-authored-by: @winskuo-quic, @chenweng-quic
Followup PRs
Test plan
pytest backends/qualcomm/tests/rework/htp/op/v68/test.py --tb=nopytest backends/qualcomm/tests/rework/htp/op/v68/test.py -k "test_conv2d" --test_report test_report.xmlpytest backends/qualcomm/tests/rework/htp/op/v68/test.py -k "test_mean & 8a"pytest backends/qualcomm/tests/rework/htp/feature/v68/test.py --soc_model SM8850 --build_folder build-android --device $DEVICE_SERIALcc @cccclai @winskuo-quic @shewu-quic @DannyYuyang-quic @cbilgin @abhinaykukkadapu