Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a dedicated CI workflow intended to exercise TopK (and a related pnnx ONNX TopK test) across multiple Linux build configurations, along with minor formatting-only adjustments in a few C++ sources.
Changes:
- Added a new GitHub Actions workflow to build and run
test_topkunder several x86/x64 feature configurations and runctest -R test_onnx_torch_topkfor pnnx. - Formatting-only indentation changes in pnnx ONNX constant folding and Python export code paths.
- Indented OpenMP
#pragmalines inTopK::forward().
Reviewed changes
Copilot reviewed 1 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tools/pnnx/src/pass_onnx/fold_constants.cpp | Formatting-only indentation change in op-type whitelist condition. |
| tools/pnnx/src/ir.cpp | Formatting-only indentation changes in a few multi-line conditional blocks. |
| src/layer/topk.cpp | Indented OpenMP pragmas (no functional change). |
| .github/workflows/topk-linux-test.yml | New workflow to build/test TopK and run a pnnx ONNX TopK test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| on: | ||
| push: | ||
| branches: | ||
| - topk-ci-tests |
There was a problem hiding this comment.
The on.push.branches list items are indented at the same level as the branches: key, which makes this workflow invalid YAML and prevents GitHub Actions from loading it. Indent the - topk-ci-tests entry under branches: (as done in other workflows) so the trigger is parsed correctly.
| - topk-ci-tests | |
| - topk-ci-tests |
| x64-none: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: build |
There was a problem hiding this comment.
Each steps: block has its list items (- uses:, - name:, etc.) indented at the same level as the steps: key, which is invalid YAML. The list items need to be indented under steps: (e.g., two additional spaces) or the workflow will fail to parse.
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: build |
There was a problem hiding this comment.
This workflow uses older GitHub Actions versions (actions/checkout@v4, actions/setup-python@v5) while the repository’s other workflows use actions/checkout@v6 and actions/setup-python@v6 (e.g., .github/workflows/linux-x64-cpu-gcc.yml:41, .github/workflows/pnnx.yml:45). Consider bumping to the repo-standard versions for consistency and latest security fixes.
| push: | ||
| branches: | ||
| - topk-ci-tests | ||
|
|
There was a problem hiding this comment.
Most workflows in this repo explicitly set minimal permissions (commonly contents: read). This workflow omits permissions, which can result in broader default token permissions depending on repo/org settings. Add an explicit permissions block to follow the repo’s security posture (see .github/workflows/linux-x64-cpu-gcc.yml:34-36).
| permissions: | |
| contents: read |
| on: | ||
| push: | ||
| branches: | ||
| - topk-ci-tests |
There was a problem hiding this comment.
The workflow is configured to run only on push to the topk-ci-tests branch. If the intent is to run these TopK checks for PRs, you’ll need to add a pull_request trigger (and likely a paths filter like other workflows) so it runs when relevant files change on PRs to the main branch.
| - topk-ci-tests | |
| - topk-ci-tests | |
| pull_request: | |
| branches: | |
| - main |
No description provided.