From e7bedfaf82e9be29167384103f110da41d1b99b2 Mon Sep 17 00:00:00 2001 From: Laurent Zuijdwijk Date: Tue, 8 Sep 2026 00:06:34 +0100 Subject: [PATCH 1/5] tests : keep the test-rocmfpx assertions alive in Release builds Every check in test_rocmfpx.c is a plain assert(), including the rocmfpx_validate_row_data_*() calls and the mse ordering comparisons. The Release configuration the PR builds defines NDEBUG, so all of them compiled away and the test still exited 0 - it validated nothing. Undefine NDEBUG before including , the way test-quantize-fns.cpp and test-quantize-perf.cpp already do. test_rocmfp2_reference.c does not need this: it uses its own always-active check machinery rather than assert(). Assisted-by: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VcXid9TGNfxqxxNWFQGES8 --- ggml/rocmfpx/test_rocmfpx.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ggml/rocmfpx/test_rocmfpx.c b/ggml/rocmfpx/test_rocmfpx.c index ff1db79fdd50..75dc6a0d614d 100644 --- a/ggml/rocmfpx/test_rocmfpx.c +++ b/ggml/rocmfpx/test_rocmfpx.c @@ -8,6 +8,11 @@ #include "rocmfpx.h" +// Every check below is an assert(), so keep them alive in Release builds - +// otherwise the test would validate nothing and still exit 0. +#ifdef NDEBUG +#undef NDEBUG +#endif #include #include #include From b19b79d2a4f9d53adc0f88e096725c83de9ea0c3 Mon Sep 17 00:00:00 2001 From: Laurent Zuijdwijk Date: Tue, 8 Sep 2026 00:06:34 +0100 Subject: [PATCH 2/5] tests : link rocmfpx from ggml-base instead of compiling a second copy rocmfpx.c is already a source of ggml-base, and everything test_rocmfpx.c calls is declared GGML_API. Compiling it into the test as well breaks MSVC shared-library builds: with BUILD_SHARED_LIBS=ON and GGML_BACKEND_DL=OFF the test inherits GGML_SHARED through llama -> ggml -> ggml-base but not GGML_BUILD, so the codec declarations become __declspec(dllimport) in the same translation unit that defines them, which MSVC rejects. Drop the extra source and let the test link the library implementation. test-rocmfp2-reference keeps compiling rocmfp2_reference.c directly: it is a standalone reference that is not part of ggml-base and carries no GGML_API. Assisted-by: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VcXid9TGNfxqxxNWFQGES8 --- tests/CMakeLists.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f52433a4a2b6..7609981116ed 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -329,8 +329,14 @@ if (NOT GGML_BACKEND_DL) # ROCmFPx codec self-tests, kept next to the codecs under ggml/rocmfpx. # test-rocmfpx exercises the ROCmFP3/6/8 codecs; test-rocmfp2-reference # checks the standalone ROCmFP2 reference encoder. + # + # rocmfpx.c is already part of ggml-base and its entry points are GGML_API, + # so link it rather than compiling a second copy: in a shared build the + # inherited GGML_SHARED would make those declarations __declspec(dllimport) + # in the very translation unit that defines them, which MSVC rejects. + # rocmfp2_reference.c is standalone and carries no GGML_API, so it is still + # compiled straight into its test. llama_build_and_test(${PROJECT_SOURCE_DIR}/ggml/rocmfpx/test_rocmfpx.c - ${PROJECT_SOURCE_DIR}/ggml/rocmfpx/rocmfpx.c NAME test-rocmfpx) target_include_directories(test-rocmfpx PRIVATE ${PROJECT_SOURCE_DIR}/ggml/rocmfpx) llama_build_and_test(${PROJECT_SOURCE_DIR}/ggml/rocmfpx/test_rocmfp2_reference.c From 463bc10e14b4e29449b809f497ef032a2f3233aa Mon Sep 17 00:00:00 2001 From: Laurent Zuijdwijk Date: Tue, 8 Sep 2026 00:06:49 +0100 Subject: [PATCH 3/5] rocmfpx : let the ROCmFP2 encoder pick the zero scale rocmfpx_choose_scale_fp2_mse() only searched scale bytes 1..126, returning 0 solely when the block was all zeros. ROCmFP2's S40 codebook is {-4, -1, +1, +4}, which has no zero code, so at any nonzero scale the smallest magnitude it can emit is one scale step. A block that sits below half of the smallest UE4M3 scale is then amplified rather than approximated. A block filled with 2^-14 reconstructed as 2^-10, 16x too large, for a squared error 225x greater than encoding it as zero. rocmfp2_p1_quantize_block_ref(), which searches every legal byte 0x00..0x7e, picked scale byte 0 for the same input - so the two encoders in this tree disagreed. Seed the search with the zero-scale candidate so bytes 1..126 have to beat it. Ties keep byte 0, matching both the lower-byte rule the rest of the search uses and the standalone reference. before: e=(1,1) sse=2.68e-05 after: e=(0,0) sse=1.19e-07 (reference: 5.96e-08 per 16-weight group) FP3, FP6 and FP8 all have a zero code and so can already match the zero scale by rounding; only FP2 needs this. Assisted-by: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VcXid9TGNfxqxxNWFQGES8 --- ggml/rocmfpx/rocmfpx.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ggml/rocmfpx/rocmfpx.c b/ggml/rocmfpx/rocmfpx.c index b169cd96a88b..99dc13231a0b 100644 --- a/ggml/rocmfpx/rocmfpx.c +++ b/ggml/rocmfpx/rocmfpx.c @@ -306,9 +306,17 @@ static uint8_t rocmfpx_choose_scale_fp2_mse( } const float * weights = quant_weights ? mse_weights : NULL; + + // The S40 codebook {-4, -1, +1, +4} has no zero code, so at any nonzero scale + // the smallest magnitude ROCmFP2 can emit is one scale step. A block that sits + // below half of the smallest UE4M3 scale is therefore encoded better by the + // zero scale than by e = 1, which would amplify every value. Seed the search + // with that candidate so scales 1..126 have to beat it; ties keep byte 0, + // matching the lower-byte rule the rest of the search uses. + uint8_t best_e = 0; + float best_err = rocmfpx_fp2_group_mse_for_scale(x, weights, n, 0, INFINITY); + const uint8_t start_e = rocmfpx_nearest_scale_ue4m3(max_abs / 4.0f); - uint8_t best_e = start_e; - float best_err = INFINITY; bool lower_done = false; for (int delta = 0; delta <= 125; ++delta) { From a6c9e7e21ba452fb7517a1e678bf0d5c234011c4 Mon Sep 17 00:00:00 2001 From: Laurent Zuijdwijk Date: Tue, 8 Sep 2026 00:07:04 +0100 Subject: [PATCH 4/5] rocmfpx : bound FP6 scale pruning by 32, the magnitude it can reach The FP6 scale search stops descending once the unavoidable clipping error at a scale already exceeds the best error found so far. That bound was computed as max_abs - 31*scale, but rocmfpx_decode_fp6_code() maps code 32 to -32: the representable range is the asymmetric [-32, 31]. For a block whose largest magnitude is negative the bound therefore overestimates the error that cannot be avoided, and the search gives up before reaching the scale that wins. Sweeping 20000 half-blocks with a single dominant outlier: before: 324 suboptimal with a negative outlier, 0 with a positive one after: 0 suboptimal in either case worst observed case: byte 12 at sse 1.783e-04 where byte 11 gives 1.367e-04. The asymmetry between the two columns is exactly what the wrong bound predicts. The Vulkan encoder carried the same bound, so fix both together - the two encoders have to agree on the scale they choose. Assisted-by: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VcXid9TGNfxqxxNWFQGES8 --- ggml/rocmfpx/rocmfpx.c | 6 +++++- ggml/src/ggml-vulkan/vulkan-shaders/copy_to_quant.comp | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/ggml/rocmfpx/rocmfpx.c b/ggml/rocmfpx/rocmfpx.c index 99dc13231a0b..de573338bc21 100644 --- a/ggml/rocmfpx/rocmfpx.c +++ b/ggml/rocmfpx/rocmfpx.c @@ -812,7 +812,11 @@ static uint8_t rocmfpx_choose_scale_fp6_mse_impl( const int e0 = (int) start_e - delta; if (!lower_done && e0 >= 1 && e0 <= 126) { const float scale = rocmfpx_scale_lookup((uint8_t) e0); - const float clip_delta = max_abs - 31.0f*scale; + // ROCmFP6 reaches -32, not just 31, so bound the unavoidable clipping + // error by 32: at 31 the bound is too pessimistic for a block whose + // largest magnitude is negative, and the search stops before reaching + // the scale that actually wins. + const float clip_delta = max_abs - 32.0f*scale; const float clip_err = mse_weights ? max_abs_weight*clip_delta*clip_delta : clip_delta*clip_delta; if (clip_delta > 0.0f && clip_err > best_err) { lower_done = true; diff --git a/ggml/src/ggml-vulkan/vulkan-shaders/copy_to_quant.comp b/ggml/src/ggml-vulkan/vulkan-shaders/copy_to_quant.comp index 481f9a8ca135..bed84cef00bc 100644 --- a/ggml/src/ggml-vulkan/vulkan-shaders/copy_to_quant.comp +++ b/ggml/src/ggml-vulkan/vulkan-shaders/copy_to_quant.comp @@ -680,7 +680,11 @@ uint rocmfpx_choose_scale_fp6_mse(uint src_idx, uint offset) { const int e0 = int(start_e) - int(delta); if (!lower_done && e0 >= 1 && e0 <= 126) { const float scale = ue4m3_to_fp32(uint8_t(uint(e0))); - const float clip_delta = max_abs - 31.0 * scale; + // ROCmFP6 reaches -32, not just 31, so bound the unavoidable clipping + // error by 32 - matching rocmfpx_choose_scale_fp6_mse_impl() on the CPU. + // At 31 the bound is too pessimistic for a block whose largest magnitude + // is negative and the search stops before the scale that actually wins. + const float clip_delta = max_abs - 32.0 * scale; if (clip_delta > 0.0 && clip_delta*clip_delta > best_err) { lower_done = true; } else { From 9bfeb35396bbada63fa0b37f7422499b83cb8e00 Mon Sep 17 00:00:00 2001 From: Laurent Zuijdwijk Date: Tue, 8 Sep 2026 00:07:21 +0100 Subject: [PATCH 5/5] rocmfpx : clamp before the integer conversion in the FP6/FP8 encoders rocmfpx_quantize_fp6_code(), rocmfpx_fp6_decoded_value() and rocmfpx_quantize_fp8_code() rounded to int first and clamped afterwards. The isfinite() guard keeps NaN and inf out, but a large finite weight still overflows the conversion, so the value that gets clamped is already wrong: 1e12 -> fp6 -0.03125 sign flipped 1e30 -> fp6 0, fp8 0 magnitude lost entirely Clamp the float to the representable range first, then convert. For in-range values this is identical to the old order - rounding cannot push a value past a bound it was not already past - so ordinary weights encode bit for bit as before, and the extremes now saturate: 1e12 -> fp6 31*scale, fp8 127*scale 1e30 -> fp6 31*scale, fp8 127*scale The Vulkan encoder had the same shape, and int() of an out-of-range float is undefined in GLSL, so fix it in the same commit to keep the two encoders in agreement. This is extreme-input robustness, not a defect seen on ordinary weights. Assisted-by: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VcXid9TGNfxqxxNWFQGES8 --- ggml/rocmfpx/rocmfpx.c | 34 +++++++------------ .../vulkan-shaders/copy_to_quant.comp | 7 ++-- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/ggml/rocmfpx/rocmfpx.c b/ggml/rocmfpx/rocmfpx.c index de573338bc21..8582a2038539 100644 --- a/ggml/rocmfpx/rocmfpx.c +++ b/ggml/rocmfpx/rocmfpx.c @@ -690,6 +690,15 @@ static uint8_t rocmfpx_choose_scale_fp3_weighted_mse(const float * x, int n, con return rocmfpx_choose_scale_fp3_mse_impl(x, n, mse_weights, max_abs, max_abs_weight, all_finite); } +// Round to the nearest integer inside [lo, hi], clamping *before* the conversion. +// Converting first and clamping afterwards overflows int (and, for large enough +// inputs, long) so an extreme finite weight could flip sign or collapse to zero. +// For in-range values this is identical to rounding then clamping. +static inline int rocmfpx_round_clamp(float v, float lo, float hi) { + const float c = v < lo ? lo : (v > hi ? hi : v); + return (int) lroundf(c); +} + static int rocmfpx_decode_fp6_code(uint8_t code) { const int mag = code & 31u; return (code & 32u) ? -(mag == 0 ? 32 : mag) : mag; @@ -700,12 +709,7 @@ static uint8_t rocmfpx_quantize_fp6_code(float x, float inv_scale) { return 0; } - int q = (int) lroundf(x * inv_scale); - if (q > 31) { - q = 31; - } else if (q < -32) { - q = -32; - } + const int q = rocmfpx_round_clamp(x * inv_scale, -32.0f, 31.0f); return q == 0 ? 0 : (uint8_t) (q < 0 ? (32u | ((uint8_t) -q & 31u)) : (uint8_t) q); } @@ -714,14 +718,7 @@ static uint8_t rocmfpx_quantize_fp6_code(float x, float inv_scale) { // current main's asymmetric signed range [-32, 31], including the encoded -32 // endpoint, rather than the older experimental branch's [-31, 31] behavior. static inline float rocmfpx_fp6_decoded_value(float x, float inv_scale) { - int q = (int) lroundf(x * inv_scale); - if (q > 31) { - q = 31; - } else if (q < -32) { - q = -32; - } - - return (float) q; + return (float) rocmfpx_round_clamp(x * inv_scale, -32.0f, 31.0f); } static float rocmfpx_fp6_block_mse_for_scale(const float * x, int n, uint8_t e, float best_err) { @@ -890,14 +887,7 @@ static int8_t rocmfpx_quantize_fp8_code(float x, float inv_scale) { return 0; } - int q = (int) lroundf(x * inv_scale); - if (q > 127) { - q = 127; - } else if (q < -127) { - q = -127; - } - - return (int8_t) q; + return (int8_t) rocmfpx_round_clamp(x * inv_scale, -127.0f, 127.0f); } static float rocmfpx_fp8_block_weighted_mse_for_scale(const float * x, int n, const float * mse_weights, uint8_t e, float best_err) { diff --git a/ggml/src/ggml-vulkan/vulkan-shaders/copy_to_quant.comp b/ggml/src/ggml-vulkan/vulkan-shaders/copy_to_quant.comp index bed84cef00bc..a0707c145958 100644 --- a/ggml/src/ggml-vulkan/vulkan-shaders/copy_to_quant.comp +++ b/ggml/src/ggml-vulkan/vulkan-shaders/copy_to_quant.comp @@ -629,7 +629,9 @@ uint rocmfpx_fp6_quantize_code(float x, float inv_scale) { } // asymmetric signed range [-32, 31]; -32 is encoded as sign|0 - const int q = clamp(int(round(x * inv_scale)), -32, 31); + // Clamp in float before the int conversion: int() of an out-of-range float is + // undefined in GLSL, so an extreme finite weight could flip sign or vanish. + const int q = int(clamp(round(x * inv_scale), -32.0, 31.0)); if (q == 0) { return 0u; } @@ -746,8 +748,7 @@ int rocmfpx_fp8_quantize_code(float x, float inv_scale) { return 0; } - int q = int(round(x * inv_scale)); - return clamp(q, -127, 127); + return int(clamp(round(x * inv_scale), -127.0, 127.0)); } void quantize(uint dst_idx, uint src_idx)