Skip to content

Fix fp quantized matvec for output dim < 8#3804

Open
katlun-lgtm wants to merge 1 commit into
ml-explore:mainfrom
katlun-lgtm:fix/mxfp4-qmv-small-outdim
Open

Fix fp quantized matvec for output dim < 8#3804
katlun-lgtm wants to merge 1 commit into
ml-explore:mainfrom
katlun-lgtm:fix/mxfp4-qmv-small-outdim

Conversation

@katlun-lgtm

Copy link
Copy Markdown
Contributor

Summary

fp_qmv_impl (the Metal quantized_matmul matvec path, M == 1) has an
out_vec_size < 8 branch that is taken when the weight matrix has fewer than
one full output tile. Its full-block loop read the fp8 group scale as a raw
byte and passed it straight to qdot:

uint8_t s = sl[0];                       // raw 0-255 byte
result[row] += qdot<U, values_per_thread, bits>(wl, x_thread, s);

Every sibling site decodes the scale first -- including the remainder loop of
this same branch (and the other loops in fp_qmv_impl):

U s = dequantize_scale<U, group_size>(sl[0]);

qdot multiplies by s directly, so the undecoded byte corrupts the result
for any fp quantization mode (mxfp4 / mxfp8 / nvfp4) whose output dimension is
< 8. Affine quantization uses different kernels and is unaffected.

Fixes #3762.

Repro (M3 Max, Metal)

x @ dequantize(w).T vs quantized_matmul, mxfp4, group_size=32, K=512:

output dim before after
5 rel_err 2.18e+02 rel_err 3.01e-08
7 rel_err 1.83e+02 rel_err 1.45e-07
8 rel_err 2.03e-07 rel_err 2.03e-07
12 rel_err 7.83e-08 rel_err 7.83e-08
16 rel_err 6.74e-08 rel_err 6.74e-08

Affine control is ~1e-7 throughout, unchanged.

Testing

  • Extended test_fp_qmv with output dims M = 5, 7, which exercise the
    previously-untested out_vec_size < 8 path.
  • python -m pytest python/tests/test_quantized.py: 32 passed, 2896 subtests.

@katlun-lgtm

Copy link
Copy Markdown
Contributor Author

Independently reproduced and verified this fix on an Apple M3 Max (Metal), building from source at this branch's base:

Before (unpatched main)mxfp4 matvec, group_size=32, K=512:

N rel_err
5 2.180e+02 WRONG
7 1.833e+02 WRONG
8, 12, 16 ~1e-7 ok

After this one-line change (uint8_t s = sl[0]U s = dequantize_scale<U, group_size>(sl[0])):

N rel_err
5 3.011e-08 ok
7 1.445e-07 ok
8, 12, 16 ~1e-7 ok

The affine (integer) control path is unchanged (~1e-7) before and after. Full suite: python -m pytest python/tests/test_quantized.py32 passed, 2848 subtests passed. LGTM.

@katlun-lgtm katlun-lgtm closed this Jul 5, 2026
@katlun-lgtm katlun-lgtm reopened this Jul 5, 2026
@katlun-lgtm

Copy link
Copy Markdown
Contributor Author

The failing macOS checks here look unrelated to this change.

On the macOS jobs the MLX test suite itself passes:

Ran 773 tests in 90.930s

OK (skipped=5)

and Linux (CPU + all CUDA variants), lint, and docs are green. The macOS job then fails in the separate mpi_test_distributed.py step with a segfault inside OpenMPI/PRRTE at MPI init — before any MLX code runs:

[...] *** Process received signal ***
[...] Signal: Segmentation fault: 11 (11)
[...] libprrte.3.dylib   construct_range + 36
[...] libprrte.3.dylib   prte_hwloc_base_get_topo_signature + 632
[...] libprrte.3.dylib   rte_init + 1940
[...] libprrte.3.dylib   prte_init + 1044
93754 Segmentation fault: 11  mpirun --bind-to none -host localhost:8 -np 8 -x DYLD_LIBRARY_PATH=/opt/homebrew/lib/ python python/tests/mpi_test_distributed.py
##[error]Process completed with exit code 139

The crash is in the hwloc topology probe during prte_init, so it looks like an OpenMPI/runner-environment issue rather than anything in this quantized-matvec change — all three macOS runners (14/15/26) hit it identically in the same run, while the actual MLX tests (including the new out_vec_size < 8 cases) pass.

I've reopened the PR to re-run CI; the new run is currently waiting on workflow approval. Happy to rebase if that's easier.

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the nice fix! The CI problem is not related to this PR and I will take a look at it.

@zcbenz zcbenz changed the title Fix fp quantized matvec for output dim < 8 (#3762) Fix fp quantized matvec for output dim < 8 Jul 5, 2026
@katlun-lgtm

Copy link
Copy Markdown
Contributor Author

For whoever picks up the macOS CI fix — this looks like a known OpenMPI 5.0.8 PRRTE bug rather than anything specific to this PR: open-mpi/ompi#13357.

The crash in the mpi_test_distributed.py step is a NULL-pointer deref inside PRRTE's hwloc topology code at launch:

Signal: Segmentation fault: 11
libprrte.3.dylib   prte_hwloc_base_get_topo_signature
libprrte.3.dylib   rte_init / prte_init

which lines up with #13357: prte_node_topologies can hold an entry whose t->topo == NULL (only a signature was added), and prte_hwloc_base_setup_summary() / prte_hwloc_base_filter_cpus() then dereference it. It's intermittent — depends on how the topology array gets populated — which fits it passing on some runs and segfaulting on others.

Since it's in the PRRTE bundled with openmpi 5.0.8, options on the (self-hosted) runner side: pin/downgrade Homebrew openmpi below 5.0.8, rebuild --with-hwloc=internal, or add a retry on the mpirun step. Happy to help if useful.

fp_qmv_impl's out_vec_size < 8 branch (taken when the weight matrix has
fewer than one full output tile) read the fp8 group scale as a raw byte
in its full-block loop and passed it straight to qdot, instead of
decoding it with dequantize_scale like every sibling site -- including
the remainder loop of the same branch. This corrupted mxfp4/mxfp8/nvfp4
matvec whenever the output dimension is < 8 (rel_err ~2e2). Affine
quantization uses different kernels and is unaffected.

Also extend test_fp_qmv with small output dims (M = 5, 7) which exercise
the previously-untested out_vec_size < 8 path.
@angeloskath angeloskath force-pushed the fix/mxfp4-qmv-small-outdim branch from b091549 to a0aa251 Compare July 6, 2026 18:16
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.

Metal fp_qmv_impl: out_vec_size < 8 branch uses raw scale byte instead of dequantize_scale → wrong mxfp4 matvec for output dim < 8

2 participants