ggml : update ggml_prec specification - #26675
Conversation
| enum ggml_prec prec); | ||
|
|
||
| // set the smallest rank that the implementation can use to internally convert the src1 data to | ||
| // ranks in decreasing order: |
There was a problem hiding this comment.
I think it will be difficult to strictly order these and have such coarse groupings, they are probably not so interchangeable. For example, if something can be quantized to MXFP4 is it really also safe to quantize to Q4_0? It's probably model-dependent. Would the gguf be able to encode that distinction?
I'd like to eventually see FP8 (E4M3) as an option, but I imagine it would not be interchangeable with Q8_1.
There was a problem hiding this comment.
I'd like to eventually see FP8 (E4M3) as an option
+1, I have some WIP for FP8 (E4M3) support lying around somewhere 😃
There was a problem hiding this comment.
It's probably model-dependent. Would the gguf be able to encode that distinction?
Yes, the proposal in #24364 is to include information in the GGUF:
I think the decision in the backend for which 4-bit quantization to use for the src1 should be driven by the src0 type (in addition to the provided ggml_prec for src1).
So for example, if we have a weight tensor in MXFP4/NVFP4 without any additional information about it, the backend will see GGML_PREC_DEFAULT (or GGML_PREC_UNDEFINED if we rename it) and it would likely quantize src1 to MXFP4/NVFP4 (if there is hardware support). There is no reason to quantize to Q4_0 if src0 is MXFP4/NVFP4.
If the model provider decided that FP4 x FP4 is poor quality for that tensor, then the model meta info will provide the optional policy to use GGML_PREC_Q8 for this weight. In this case the llama.cpp context will call ggml_mul_mat_set_prec_src1(x, GGML_PREC_Q8) and the backends will know to not quantize to any 4-bits, even if there is hardware support.
If the weight tensor is any other type (Q4_0, Q8_0, F16 for example), then without additional information, the backend would technically be allowed to quantize src1 to Q4_0. But from experience we know this is not beneficial so no need to do it. The user code (for example llama.cpp) can also be explicit and add a default policy for non-MXFP4/NVFP4 types to set src1 prec to GGML_PREC_Q8 - this way it's unambiguous that the backend should not quantize to Q4_0.
I think these apply to FP8, but let me know if you think of a specific case that would not be clear. I.e., unless the weight is FP8 and there is hardware support for FP8, then the backend would know there is not won't have a reason to prefer FP8 over Q8_1.
ORippler
left a comment
There was a problem hiding this comment.
What's the plan to test correct behavior of a backend? Numerical precision/accuracy + test-backend-ops?
| GGML_PREC_DEFAULT = 0, // stored as ggml_tensor.op_params, 0 by default | ||
| GGML_PREC_DEFAULT = 0, // TODO: rename to GGML_PREC_UNDEFINED? | ||
| GGML_PREC_F32 = 10, | ||
| GGML_PREC_F16 = 20, |
There was a problem hiding this comment.
BF16 was introduced as FP16 shows numerical issues in Deep Learning applications, and is the de facto standard for LLM (pre-)training. So we may potentially look to discriminate between FP16 and BF16 here
There was a problem hiding this comment.
So think about this enum such that it does not necessarily correspond to a specific data type. It would likely be better to name it like this:
enum ggml_prec {
GGML_PREC_UNDEFINED = 0,
GGML_PREC_32_BITS = 10,
GGML_PREC_16_BITS = 20,
GGML_PREC_8_BITS = 30,
GGML_PREC_4_BITS = 40,
};This way, the same consideration as I explained earlier applies. If the src0 is BF16 then the backend has no reason to consider converting src1 to FP16. If the src0 is Q8_0 for example, respectively there is no reason to convert the src1 to BF16 (the scale factor of Q8_0 is FP16).
I considered the BF16 vs FP16 distinction here, but I still don't see a case where it would really be needed.
There was a problem hiding this comment.
If the src0 is Q8_0 for example, respectively there is no reason to convert the src1 to BF16 (the scale factor of Q8_0 is FP16).
There is: weight-only quantization (which sparked this PR/debate). For NVFP4 W4A16, the approach would look roughly like this
// x is NVFP4 (W4)
// y is supposed to be A16 (and by virtue of BF16 being training standard most likely BF16 over FP16)
ggml_tensor * z = ggml_mul_mat(ctx, x, y);
ggml_mul_mat_set_prec_acc (z, GGML_PREC_F32);
ggml_mul_mat_set_prec_src1(z, GGML_PREC_BF16);
I considered the BF16 vs FP16 distinction here, but I still don't see a case where it would really be needed.
Numerical stability/exactness (the FP32 we materialize in ggml is expected to reside in BF16's representable value ranges for LLMs as opposed to F16 value range, as LLMs were trained in BF16). But those are most likely nits/someone would need to do a research study on this
There was a problem hiding this comment.
Maybe an extra enum member GGML_PREC_WIDE_RANGE which can differentiate and help signal between BF16, FP16, MXFP8, FP8(E4M3, E5M2)
There was a problem hiding this comment.
What i mean is for storing BF16 we can have "GGML_PREC_16_BITS | GGML_PREC_WIDE_RANGE" and for FP16 "GGML_PREC_16_BITS" only . We can check GGML_PREC_16_BITS for common case signals and GGML_PREC_WIDE_RANGE signal for specific case when we need wide range
There was a problem hiding this comment.
@ORippler Specifically for NVFP4, if the hint for src1 is GGML_PREC_F16, then the backend can easily deduce to use BF16 over FP16 for the representation of src1. I don't think having explicit GGML_PREC_F16 + GGML_PREC_BF16 would help in this case.
There was a problem hiding this comment.
@ORippler Specifically for NVFP4, if the hint for src1 is GGML_PREC_F16, then the backend can easily deduce to use BF16 over FP16 for the representation of src1. I don't think having explicit GGML_PREC_F16 + GGML_PREC_BF16 would help in this case.
How would a backend deduce this intention? 🤔 I'm fine with it being undefined/open to the backend, was just wondering if it would make sense to precise it at this granularity (we have had "silent" FP16 overflows in the past iirc)
There was a problem hiding this comment.
@ORippler Hm, yes - it can't deduce (I somehow thought that FP16 cannot represent all NVFP4 values exactly).
There was a problem hiding this comment.
I've added explicit GGML_PREC_BF16. Later on we can also add GGML_PREC_FP8 if see it's necessary.
| // for example: | ||
| // - GGML_PREC_DEFAULT - the implementation is allowed to choose the most efficient accumulation type (i.e. same as GGML_PREC_F16) | ||
| // - GGML_PREC_F32 - requires accumulation of the results in F32 | ||
| // - GGML_PREC_F16 - can accumulate the results in F16, BF16, F32 |
There was a problem hiding this comment.
Same as above regarding FP16 & BF16. NVGPUs can only accumulate in F16/F32 though
|
Note that in CUDA there is currently no way to force F32 accumulation for FA |
That should be OK. The CUDA backend currently has a mechanism to increase the numerical range of the accumulators: #17746. For example, if a FA op receives: ggml_flash_attn_ext_set_prec_acc(z, GGML_PREC_F16);Then it means the CUDA backend does not have to do the |
Not 100% sure on this point, but my first thought is that the configured precisions would only affect the max err of the op in |
|
I think we can maybe just start with |
yeah I thought along those lines as well, and we are already doing this for W4A4 in NVFP4/MXFP4 tests inside |
On similar note, I think CPU FA will abort with GGML_PREC_FP16 |
| // - ggml_mul_mat_set_prec_src1(a, GGML_PREC_Q4): | ||
| // - allows the implementation to quantize F32, F16 data down to GGML_TYPE_Q8_0 or GGML_TYPE_NVFP4 | ||
| // | ||
| GGML_API void ggml_mul_mat_set_prec_src1( |
There was a problem hiding this comment.
This can be made generic something like
GGML_API void ggml_set_prec_src(struct ggml_tensor * a, int isrc, enum ggml_prec prec);
This can be used for FA(K,V, Q), mat_mul(activations) ,moe mat_mul_id(activations) . I mean src[0], src[1], src[2]
There was a problem hiding this comment.
This can be made generic something like
GGML_API void ggml_set_prec_src(struct ggml_tensor * a, int isrc, enum ggml_prec prec);
Yes, it's possible.
On similar note, I think CPU FA will abort with GGML_PREC_FP16
Currently it will abort, but we'll have to adjust it when the spec is completed.
This commit adds conversion support for NVIDIA Nemotron 3.5 Lightning. It does not include NVFP4 models, including dflash and dspark. The motivation for not including these conversion is that those checkpoint was designed as W4A16, but the converted GGUF only says that Its weights are NVFP4. On Blackwell, llama.cpp sees NVFP4 weights and will automatically selects its native FP4 path, dynamically converting activations to FP4. That makes the executed operation W4A4 which can affect the accuracy of the draft model and therefor in turn effect the acceptance rate of the target model. The references PR below has more details on a solution to address this issue. Once it has been merged we should be able to convert and publish these models, but for now we are just commenting them out. Refs: ggml-org/llama.cpp#26675
This commit adds conversion support for NVIDIA Nemotron 3.5 Lightning. It does not include NVFP4 models, including dflash and dspark. The motivation for not including these conversion is that those checkpoint was designed as W4A16, but the converted GGUF only says that Its weights are NVFP4. On Blackwell, llama.cpp sees NVFP4 weights and will automatically selects its native FP4 path, dynamically converting activations to FP4. That makes the executed operation W4A4 which can affect the accuracy of the draft model and therefor in turn effect the acceptance rate of the target model. The references PR below has more details on a solution to address this issue. Once it has been merged we should be able to convert and publish these models, but for now we are just commenting them out. Refs: ggml-org/llama.cpp#26675
89983d5 to
36b6666
Compare
| ggml_set_op_params_i32(a, 3, prec_i32); | ||
| } | ||
| break; | ||
| default: |
There was a problem hiding this comment.
Can we also handle MUL_MAT_ID here?
| ggml_set_op_params_i32(a, 2 + idx, prec_i32); | ||
| } | ||
| break; | ||
| default: |
There was a problem hiding this comment.
same, can we handle MUL_MAT_ID here as well
|
|
||
| ggml_set_op_params_i32(a, 0, prec_i32); | ||
| } | ||
| break; |
There was a problem hiding this comment.
looks like break got dropped mistakenly here?
There was a problem hiding this comment.
Yes, my mistake - thank you.
Signed-off-by: ynankani <ynankani@nvidia.com>
| }; | ||
|
|
||
| // precision | ||
| // this enum is used to declare the allowed floating-point types that can be used during the compute of an op |
There was a problem hiding this comment.
| // this enum is used to declare the allowed floating-point types that can be used during the compute of an op | |
| // this enum is used to declare the allowed numerical precision/data-types that can be used during the compute of an op |
Q4/Q8 are not floating point, so the comment is a bit misleading. Also, ggmL-prec is now overloaded to signal either specific data-types (FP32/BF16/FP16) and precision (8-bit, 4-bit)
| // - allows the implementation to quantize F32, BF16, F16 data of src[1] down to GGML_TYPE_Q8_0 | ||
| // - cannot quantize it down to GGML_TYPE_Q4_0 or GGML_TYPE_NVFP4 | ||
| // - ggml_prec_set_src(a, GGML_PREC_Q4, 1): | ||
| // - allows the implementation to quantize F32, BF16, F16 data of src[1] down to GGML_TYPE_Q8_0 or GGML_TYPE_NVFP4 |
There was a problem hiding this comment.
| // - allows the implementation to quantize F32, BF16, F16 data of src[1] down to GGML_TYPE_Q8_0 or GGML_TYPE_NVFP4 | |
| // - allows the implementation to quantize F32, BF16, F16 data of src[1] down to 4-bit datatypes such as GGML_TYPE_Q4_K, GGML_TYPE_NVFP4 etc. | |
| // | |
| // Note this is currently limited to src[1] of GGML_OP_MUL_MAT and GGML_OP_MUL_MAT_ID |
We currently effectively limit this to src[1] of MUL_MAT/MUL_MAT_ID. I feel it may be worth to reflect this in the descriptive comment.
| void ggml_prec_set_acc( | ||
| struct ggml_tensor * a, | ||
| enum ggml_prec prec) { | ||
| switch (a->op) { |
There was a problem hiding this comment.
In the docs we say only
// - GGML_PREC_F32 - requires accumulation of the results in F32
// - GGML_PREC_BF16 - can accumulate the results in BF16, F32
// - GGML_PREC_F16 - can accumulate the results in F16, F32
are supported. If the other values are truly not allowed, we should check prec here and error if the other values are supplied
There was a problem hiding this comment.
Yes, I was think that we should actually make this function return a bool and return false on failure. Also return false when applied to unsupported op, etc.
| { | ||
| const int32_t prec_i32 = (int32_t) prec; | ||
|
|
||
| ggml_set_op_params_i32(a, 3, prec_i32); |
There was a problem hiding this comment.
| ggml_set_op_params_i32(a, 3, prec_i32); | |
| ggml_set_op_params_i32(a, 3, prec_i32); // scale is on first pos, max_bias on second |
There was a problem hiding this comment.
Added a search tag [TAG_GGML_PREC] that should make looking up this information more simple.
8f1ee04 to
085ae2d
Compare
Overview
The discussion in #24364 has most of the background for this change. In short - this is a proposal to improve the specification of
enum ggml_precand how it is used to declare the accumulator types and the internal source data representation types used by the implementations (i.e. the ggml backends).Sample usage:
@ggml-org/maintainers PTAL at the comments in the code and let me know what you think.
Additional information
The proposal atm is backwards compatible and AFAICT it covers all concerns and remarks that we have had over time about how to communicate the types used during compute. Note that as proposed in #24364, there is going to be an user-level policy (in the
llama.cppcase, this is optional meta information stored in the GGUFs) that will be used to provide this precision information for the various ops in the model's compute graph.Requirements