Skip to content

Fix signedness warning in quantized Metal tail bounds check - #4675

Open
jrp2014 wants to merge 1 commit into
ml-explore:mainfrom
jrp2014:codex/fix-4670-metal-sign-compare
Open

jrp2014 wants to merge 1 commit into
ml-explore:mainfrom
jrp2014:codex/fix-4670-metal-sign-compare

Conversation

@jrp2014

@jrp2014 jrp2014 commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

This is a 1-line fix for #4670, following the invitation in #4670 (comment).

The proposed cast in thus PR removes all 126 warnings; compilation and pre-commit checks pass.

The python suite ran 963 tests, with two test_qmm_splitk_precision failures (float16 and bfloat16):

======================================================================
FAIL: test_qmm_splitk_precision (test_quantized.TestQuantized.test_qmm_splitk_precision) (mode='affine', dtype=mlx.core.bfloat16, M=16)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jrp/Documents/AI/mlx/mlx/python/tests/test_quantized.py", line 684, in test_qmm_splitk_precision
    self.assertLess(error.item(), 1.1 * rounding.item())
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 0.001717065810225904 not less than 0.0012367867981083692

======================================================================
FAIL: test_qmm_splitk_precision (test_quantized.TestQuantized.test_qmm_splitk_precision) (mode='affine', dtype=mlx.core.float16, M=16)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jrp/Documents/AI/mlx/mlx/python/tests/test_quantized.py", line 684, in test_qmm_splitk_precision
    self.assertLess(error.item(), 1.1 * rounding.item())
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 0.0002131553046638146 not less than 0.00015513441176153722

----------------------------------------------------------------------
Ran 963 tests in 136.771s

FAILED (failures=2, skipped=7)

I don't think that these are related to this fix, so I am ignoring them.

(For what it is worth, there seems to be a mismatch between the test’s assumptions and the kernel selected on this M5 Max:

  • For 16 input rows, MLX selects qmv_wide, although the test intends to exercise split-K.
  • That kernel reconstructs quantized weights in float32. The test’s reference first rounds those weights to float16/bfloat16, so it compares slightly different calculations.
  • Against a reference using float32 weight reconstruction, both failing cases achieve essentially the expected single-rounding accuracy.
  • Padding the same inputs to 33 rows selects split-K, and both cases meet the original test’s tolerance.
    So this looks like a test coverage/reference problem exposed by hardware-dependent dispatch, rather than split-K accumulation or the warning fix.)

@jrp2014
jrp2014 marked this pull request as ready for review October 10, 2026 17:56
@jrp2014

jrp2014 commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

#4676 Has more on the failing tests.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant