Repository navigation
Conversation
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
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.
What this changes
tests/cuda_parity.rsgains a table of every kernel symbol inffi.rswith the test that covers it, and a test that runs without a GPU and fails when a kernel has no row, a row names a kernel that no longer exists, or a row names a test that is not in the file. Filling the table in showed which kernels only ran indirectly inside the model test, soelementwise,reductionsandgemm_batchedget direct cases forsub,div, the scalar ops,neg,abs,sqrt,log,powf,max,min, the batchedtnlayout and batched backward.Two kernels turned out to have no caller at all:
transpose(the op is apermute) andtranspose_batched. Both are removed with their header lines, stubs, ffi declarations and wrappers.The cuBLAS handle now uses
CUBLAS_DEFAULT_MATHinstead ofCUBLAS_TF32_TENSOR_OP_MATH.Why
The README says every CUDA kernel is tested against the CPU path. There were 57 symbols and 16 tests, and nothing checked that the two matched. Now something does.
The first direct test of the batched
tnGEMM failed by 2.8e-4 against a 2e-4 tolerance. Against an f64 reference the CPU path is within 1.5e-7 and so is the GPU plain layout, but the GPUtnlayout was off by up to 5.5e-4 on 70 of 192 elements: cuBLAS was picking a TF32 tensor-core kernel for that shape and layout, which the handle allowed. The other parity tests passed only because cuBLAS did not pick TF32 for their shapes. TF32 is three significant digits, which is not "the last few digits" the docs promise. On an RTX 4050 it is worth 1.2x to 1.6x on large square GEMMs (2.78 vs 4.43 ms at 2048, 29 vs 36 ms at 4096). Full FP32 is the default that matches the contract; TF32 can come back as an opt-in if wanted.How it was verified
cargo testpassescargo clippy --all-targets -- -D warningsis cleancuda_parityon a GPU--releaseAll 17 parity cases pass on an RTX 4050 under full FP32, the new coverage test passes on the CPU build,
cargo docwith-D warningsis clean, and each of the three commits is green on its own.Checklist
tensor/opsandautograd/opsstill mirror each other///docs