cuda: declare the qwen3_5 backend - #77
Draft
xiaoyu-xyz wants to merge 5 commits into
Draft
xiaoyu-xyz wants to merge 5 commits into
xiaoyu-xyz wants to merge 5 commits into
Conversation
ThinkFlowLab#19 added the kernels under src/backends/cuda/qwen3_5/ but left them undeclared, so the directory has kernel sources and no manifest. ThinkFlowLab#25 defines the manifest and its checker, and running that checker against main reports exactly that: [error] qwen3_5: has kernel sources (attention.cu, common.cuh, elementwise.cu) but no qwen3_5.backend.json manifest This adds the declaration, with the values the merged files already state: - sources: the nine kernel and header files, from the directory listing. - build.script and build.output: what build.sh actually writes, libqwen3_5_cuda.so. - build.default_arch 89 and build.architectures [89]: build.sh defaults `arch` to CUDA_COMPUTE_CAP-or-89 and passes it to a single -gencode flag. - build.min_capability 80: the README says "Tensor-core kernels need sm_80 or newer". status is `experimental`, not `validated`. ThinkFlowLab#19 validated the kernels, but this manifest is written by someone other than their author and a `validated` manifest asserts a tolerance and a reference entrypoint, which are twu3202's to state. A declaration that the backend exists and how it builds is useful on its own and does not put words in anyone's mouth; promoting it is a one-line change once the tolerance is recorded. Checked with the checker from ThinkFlowLab#25: `1 backend(s), 0 error(s), 0 warning(s)`, and with discover_buildable.py from #2, which reports it as compile-checkable.
The manifest said abi_version 1. ThinkFlowLab#19's ops.h defines CS1_ABI_VERSION 4, so the declaration disagreed with the library it describes. Nothing caught it when this was written, because the checker did not read headers. ThinkFlowLab#25 now does: it reads the `#define <PREFIX>_ABI_VERSION N` out of the declared sources and requires the manifest to match, which is what surfaced this. abi_version is 1 but ops.h defines CS1_ABI_VERSION 4 also strengthens the description in the same PR.
Nothing ran `src/backends/cuda/tests` or `check_contract.py`: the only Python discovery in this workflow is `tests/benchmarks`. Two things make this land here rather than with the checker. The step needs `check_contract.py`, which arrives in ThinkFlowLab#25. And the checker exits 1 on a tree that has kernels but no declaration for them, which is main's state until the manifest in this branch lands — so wiring it up first would redden main. Both are stated in the job's comment and in its failure message, so a wrong merge order is self-explanatory rather than a bare "file not found". No CUDA toolkit and no GPU: the checker and its tests read files as text.
The first version ran `discover -s src/backends/cuda/tests`, which finds the contract checker's tests and nothing else. Kernel tests sit beside their kernels -- `src/backends/cuda/scoring/test_scoring.py` is 17 of them -- so a backend that merges later would have had no test coverage at all while appearing to be covered. The step now loops over `src/backends/cuda/*/` and discovers in any directory holding `test_*.py`. `-t` has to point at the directory itself rather than the repository root: the kernel directories are not packages, so `-t .` fails with "Start directory is not importable". Simulated on a tree carrying both: 17 from `scoring/`, 66 from `tests/`.
The job failed on its own branch, in five seconds:
ImportError: Start directory is not importable: 'src/backends/cuda/tests'
The checker and its tests live in ThinkFlowLab#25, so on ThinkFlowLab#77's tree there is nothing to run.
The guard I wrote checked for `check_contract.py` alone and ran *after* the test
step, so the first step failed before the guard was reached and the message
explaining the dependency never printed.
Now a first step decides whether the prerequisites are present and the three
steps run only if they are; otherwise the job reports a notice naming ThinkFlowLab#25 and
passes. That is what makes either merge order work: green on this branch before
ThinkFlowLab#25 lands, and running by itself once it does.
Verified both ways: with ThinkFlowLab#25's files absent all three steps skip, and with them
present 66 checker tests, 66 kernel tests and the contract check all pass.
6 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
#19 added the kernels under
src/backends/cuda/qwen3_5/without declaring them, so the directory has kernel sources and no manifest. #25 defines the manifest and its checker, and againstmainthe checker reports exactly that:Three commits:
build.shwriteslibqwen3_5_cuda.soand defaultsarchtoCUDA_COMPUTE_CAP-or-89 in a single-gencodeflag, and the directory README says the tensor-core kernels need sm_80 or newer.abi_version: 1;ops.hdefinesCS1_ABI_VERSION 4. Nothing caught that until cuda: add the backend contract and its checker #25's checker learned to read the header macro, which is what surfaced it.ci.ymlgains acuda-contractjob runningsrc/backends/cuda/testsandcheck_contract.py --repo-root .. No CUDA toolkit, no GPU: both read files as text.statusisexperimental, notvalidated. #19 validated the kernels, but avalidatedmanifest asserts a tolerance and a reference entrypoint, and those are twu3202's to state rather than mine. Promoting it once a tolerance is recorded is a one-line change.Merge order
This depends on #25, and the CI step needs both. The checker exits 1 on a tree with kernels and no declaration, which is
main's state without this manifest — so the CI step cannot land before it. And the step runssrc/backends/cuda/testsandcheck_contract.py, both of which arrive in #25, so it cannot land before that either. The job comments say so, and its failure message names #25 rather than reporting a bare missing file.Test Result
Simulating the two CI steps against
#25@6b3f0e6+#77@67c7b65:66 tests, OKandcheck_contractexit 0 with1 backend(s), 0 error(s), 0 warning(s).The manifest itself: parses, all nine declared sources exist, the directory holds no source file the manifest omits, and the ABI check cross-checks
ops.h'sCS1_ABI_VERSION 4againstabi_version: 4.Deliberately not included:
numericsandreference. Both are about a parity claim this does not make.