Repository navigation
cpu: add F16 SWIGLU_OAI reference implementation and validation - #29200
Conversation
github-actions
Bot
added
testing
|
Could you kindly help to review this PR? Thanks in advance! |
|
I'm going to close this for a sec to undraft the PR for F16 activations |
|
@cqderek good that we split the test :) see the CI failures. Let's try to fix those. |
1682f26 to
932bc47
Compare
|
Fixed the CI failures, please help to review again, thx. |
|
Looks good to me. |
|
You may want to add check in cuda backend. @ggml-org/ggml-cuda Unless F16 is worth adding support for? |
|
Sure, we're looking at various intermediate activations apart from f32. bf16/f16 support is welcome |
cqderek
changed the title
932bc47 to
2c8da6a
Compare
CUDA F16 support is outside the scope of this CPU reference and backend-validation change; it can be handled separately. |
|
@CISC @max-krasnyansky Please help to review again, thanks. |
2c8da6a to
45978ea
Compare
|
@CISC I removed the GLU, ABS, and LOG cases from the function "make_test_cases_perf()", please help to review again, thanks. |
cqderek
changed the title
cqderek
changed the title
|
Since you haven't ticked the box to allow us to push changes you will have to modify this so that CIs won't fail while |
45978ea to
ad770af
Compare
01930f7 to
4e8ab78
Compare
|
do not do unrelated Vulkan changes. That CI failure is covered by #30145 |
4e8ab78 to
5332b0f
Compare
…p test coverage Adds ggml_compute_forward_swiglu_oai_f16() to the CPU backend, mirroring the existing F32 path, and extends test-backend-ops with F16 test cases for SWIGLU_OAI so downstream backends(e.g. Hexagon) can be CPU-diffed for correctness.
5332b0f to
6ead3ef
Compare
CISC
added
the
merge ready
|
cc/ @ggml-org/ggml-openvino @ggml-org/ggml-openvino-triage OpenVINO fails, probably needs a check as well: |
|
@cqderek : Could you please cherry-pick the fix for OpenVINO backend : cc: @CISC |
translate_glu_swiglu_oai creates alpha_const and one as f32 constants
unconditionally, so f16 inputs hit a type mismatch in Swish/Add on GPU
("Swish inputs must have the same type but they are: f16 and f32").
translate_glu_swiglu_clamp already upcasts to f32 and converts back at
the end; apply the same pattern here.
Done |
kernel_swiglu_oai is f32-only, but supports_op accepted f16 too, so an f16 SWIGLU_OAI read its inputs as f32: wrong values and a write past the output (the f16 test-backend-ops cases from ggml-org#29200 catch both).
supports_op accepted f16 SWIGLU_OAI, but only an f32 kernel existed, so the f16 inputs were read as f32: wrong values and a write past the output (the f16 test-backend-ops cases from ggml-org#29200 catch both). Add kernel_swiglu_oai_f16, which, like swiglu_clamp_f16, loads half, computes in float and stores half.
…-org#29200) * cpu: add F16 SWIGLU_OAI reference implementation and F16 activation op test coverage Adds ggml_compute_forward_swiglu_oai_f16() to the CPU backend, mirroring the existing F32 path, and extends test-backend-ops with F16 test cases for SWIGLU_OAI so downstream backends(e.g. Hexagon) can be CPU-diffed for correctness. * ggml-openvino: fix f16 SWIGLU_OAI type mismatch in alpha/one constants translate_glu_swiglu_oai creates alpha_const and one as f32 constants unconditionally, so f16 inputs hit a type mismatch in Swish/Add on GPU ("Swish inputs must have the same type but they are: f16 and f32"). translate_glu_swiglu_clamp already upcasts to f32 and converts back at the end; apply the same pattern here. --------- Co-authored-by: ravi9 <ravi.panchumarthy@intel.com>
…-org#29200) * cpu: add F16 SWIGLU_OAI reference implementation and F16 activation op test coverage Adds ggml_compute_forward_swiglu_oai_f16() to the CPU backend, mirroring the existing F32 path, and extends test-backend-ops with F16 test cases for SWIGLU_OAI so downstream backends(e.g. Hexagon) can be CPU-diffed for correctness. * ggml-openvino: fix f16 SWIGLU_OAI type mismatch in alpha/one constants translate_glu_swiglu_oai creates alpha_const and one as f32 constants unconditionally, so f16 inputs hit a type mismatch in Swish/Add on GPU ("Swish inputs must have the same type but they are: f16 and f32"). translate_glu_swiglu_clamp already upcasts to f32 and converts back at the end; apply the same pattern here. --------- Co-authored-by: ravi9 <ravi.panchumarthy@intel.com>
Overview
Add the CPU F16 reference implementation for "SWIGLU_OAI".
This implementation mirrors the existing F32 path and provides a CPU reference for validating downstream backend implementations.
The related test changes provide correctness validation for the F16 "SWIGLU_OAI" path. They are not intended to be performance benchmarks.
Requirements
I have read and agree with the contributing guidelines
AI usage disclosure: AI tools were used to assist with code review and documentation. I reviewed and verified all submitted changes.