Skip to content

Conversation

@Acly
Copy link
Collaborator

@Acly Acly commented Aug 18, 2025

Small change to support OP_CONV_2D_DW with F16 weights like most other ops.

@Acly Acly requested a review from 0cc4m as a code owner August 18, 2025 12:18
@github-actions github-actions bot added Vulkan Issues specific to the Vulkan backend ggml changes relating to the ggml tensor library for machine learning labels Aug 18, 2025
@0cc4m
Copy link
Collaborator

0cc4m commented Aug 21, 2025

Thank you! Looks good, but to make sure the results are correct, can you add f16 tests to test-backend-ops?

Edit: If I manually set the test's kernel tensor to f16 it fails all tests, but not sure if I overlooked a step.

@Acly
Copy link
Collaborator Author

Acly commented Aug 21, 2025

CPU doesn't actually support F16 for this OP, which makes it difficult to test with backend-ops
(I think it doesn't check the type anywhere so you just get wrong results because F16 data is interpreted as F32)

There is a separate test-conv2d-dw test in ggml repo I wanted to use, it checks against a reference implementation. But those ggml tests don't seem to exist in llama.cpp repo, and PRs in ggml directly are discouraged...

I'm open for ideas, maybe there's an easy way to check F16 results against the F32 version that I missed. I'm actively using those OPs here but there's always a chance I mess up during upstreaming so a test would be nice.

@0cc4m
Copy link
Collaborator

0cc4m commented Aug 21, 2025

Fair enough, I didn't see that. The missing type check seems like an oversight. Maybe we can add support for f16 on CPU later and then add the test. Considering you didn't change the shader and the fp32 version works fine, the code should be fine.

vision.cpp looks like a neat project! Let me know if there's more things that can be done with the backend to enable or optimize for those models. Most of our issue and performance reports come from LLMs, some from Stable Diffusion, cool to see the backend being used for more kinds of models.

@0cc4m 0cc4m merged commit 97ae596 into ggml-org:master Aug 21, 2025
46 of 47 checks passed
qnixsynapse pushed a commit to janhq/llama.cpp that referenced this pull request Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ggml changes relating to the ggml tensor library for machine learning Vulkan Issues specific to the Vulkan backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants