Make GGML_SYCL_F16=ON the default - #23996
Conversation
Signed-off-by: Todd Malsbary <todd.malsbary@intel.com>
Signed-off-by: Todd Malsbary <todd.malsbary@intel.com>
|
Hi @malsbat, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
Signed-off-by: Todd Malsbary <todd.malsbary@intel.com>
There was a problem hiding this comment.
SYCL support fp32 and fp16 for two reasons:
- fp32 will provide better accuracy than fp16.
- fp32 show better performance than fp16 in some cases.
I agree to recommend to use fp16 as default.
But not change the default value of parameter GGML_SYCL_F16 and the default values in script: That will impact existed CI in all user development.
|
@arthw, thanks for reviewing. My measurements are quite different than what you're seeing - I see better performance with fp16 and no meaningful degradation in quality. Can you tell me which models and devices you're seeing better results with fp32? I'd like to add these to my local benchmarks if possible. |
|
@malsbat We can see the PP of FP16 is about double performance of FP32, but the TG is similar (FP32 is little bigger than FP16 in more cases). I think FP16 has half bandwidth than FP32 in PP, for TG FP32 code path use fp16 data type in computing in fact. I'm considering to merge FP16 and FP32 code path to one in the future. Like PP is using FP16 and TG is using FP32. Anyway, I think recommend FP16 is good idea. Thank you! |
F16 remains explictly set for example and Dockerfile builds. Signed-off-by: Todd Malsbary <todd.malsbary@intel.com>
|
Thanks @arthw, I may have misunderstood your earlier comment. I've updated the PR to leave F32 the default in CMakeLists.txt and explicitly set F16=ON in the other locations. I did notice that https://github.com/ggml-org/llama.cpp/blob/master/ci/run.sh#L113 explicitly sets F16=ON, so I'm unsure of your comment about impacting existing CI. Please let me know if this PR matches what you intended. The main change I want is to the Dockerfile so that end-users get the best performance when using the prebuilt images. |
|
@malsbat ci/run.sh - In fact, SYCL backend only CI is only building, not execute this script. build.sh and win-build-sycl.bat: I hope keep the default behavior (fp32), so that the existed CI (different users locally) won't be impacted. I'm considering to merge fp32 and fp16 code paths in different function/OPs, to get the best performance. So we won't need to modify more on this issue now. Thank you! |
|
Should I close this PR then? |
|
No! :) Thank you! |
|
Thanks for clarifying, I will update this PR shortly. |
Signed-off-by: Todd Malsbary <todd.malsbary@intel.com>
|
Updated, please review when you can |
|
@arthw Are you sure you want to make this change - F16 accumulators will definitely overflow in some cases, so generally not recommended. |
@ggerganov I think FP16 will be chosen by most user if there is no such overflow issue. This PR is used to change to FP16 in docker building and recommend FP16 in guide. In same time, the docker user will feedback the overflow issue as docker use FP16 as default. Another issue, I'm considering to merge FP32 and FP16 in SYCL backend: choose the performance better part in kernel. Thank you! |
* Add -cl-fp32-correctly-rounded-divide-sqrt to F16=ON builds Signed-off-by: Todd Malsbary <todd.malsbary@intel.com> * Make GGML_SYCL_F16=ON the default Signed-off-by: Todd Malsbary <todd.malsbary@intel.com> * Leave F32 the default F16 remains explictly set for example and Dockerfile builds. Signed-off-by: Todd Malsbary <todd.malsbary@intel.com> * Revert changes to examples/sycl/build scripts Signed-off-by: Todd Malsbary <todd.malsbary@intel.com> --------- Signed-off-by: Todd Malsbary <todd.malsbary@intel.com>
* Add -cl-fp32-correctly-rounded-divide-sqrt to F16=ON builds Signed-off-by: Todd Malsbary <todd.malsbary@intel.com> * Make GGML_SYCL_F16=ON the default Signed-off-by: Todd Malsbary <todd.malsbary@intel.com> * Leave F32 the default F16 remains explictly set for example and Dockerfile builds. Signed-off-by: Todd Malsbary <todd.malsbary@intel.com> * Revert changes to examples/sycl/build scripts Signed-off-by: Todd Malsbary <todd.malsbary@intel.com> --------- Signed-off-by: Todd Malsbary <todd.malsbary@intel.com>
Overview
The current default of GGML_SYCL_F16 is OFF. There are significant performance gains to be had during prompt processing by setting it ON: improvement on models measured averaged 2.43x (ranged from 1.48x to 3.41x) for prompt processing. Token generation measured no improvement (average of 1.00x, ranged from 0.97x to 1.02x).
At the same time, llama-perplexity showed no meaningful degradation on the wiki.test.raw, winogrande, or hellaswag benchmarks.
Additional information
Measurements below were captured on B70.
Performance measurements
Perplexity measurements
hellaswag
wiki.test.raw
winogrande
Requirements