Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
380 changes: 375 additions & 5 deletions src/lib/mlxcel-core/src/layers.rs

Large diffs are not rendered by default.

55 changes: 52 additions & 3 deletions src/models/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,26 @@ impl QuantizationArgs {
self.bits.unwrap_or(4)
}

/// Get quantization mode, defaulting to "affine"
pub fn get_mode(&self) -> &str {
self.mode.as_deref().unwrap_or("affine")
/// Get the declared quantization mode, defaulting to `"affine"` when the
/// key is absent.
///
/// Fallible on purpose (issue #973). This helper has no production callers
/// yet and is the obvious thing for the next family that needs a declared
/// mode to reach for; handing back an unchecked `&str` would let it inherit
/// the hole silently, because the returned string goes on to
/// `quantized_matmul` / `gather_qmm` / `dequantize`, which cross the cxx
/// bridge as `UniquePtr<MlxArray>` rather than `Result`, so a mode MLX
/// cannot parse is an uncatchable abort at the first forward pass rather
/// than a load error. Returning `Result` makes the check impossible to skip
/// at the point the value is read.
///
/// The `None` case is the absent key and resolves to `"affine"`; a present
/// but empty or misspelled string is rejected. See
/// [`mlxcel_core::layers::validate_quantization_mode`].
pub fn get_mode(&self) -> Result<&str, String> {
let mode = self.mode.as_deref().unwrap_or("affine");
mlxcel_core::layers::validate_quantization_mode(mode)?;
Ok(mode)
}
}

Expand Down Expand Up @@ -309,4 +326,36 @@ mod tests {
assert_eq!(args.get_group_size(), 64);
assert_eq!(args.get_bits(), 4);
}

/// `get_mode` has no production callers and is the obvious thing for the
/// next family that needs a declared mode to reach for, so the bound belongs
/// on the signature rather than on each future caller (issue #973).
#[test]
fn get_mode_bounds_the_declared_string() {
// An absent key is "not declared" and resolves to affine.
assert_eq!(
QuantizationArgs::default().get_mode().as_deref(),
Ok("affine")
);

for mode in mlxcel_core::layers::SUPPORTED_QUANTIZATION_MODES {
let args = QuantizationArgs {
mode: Some(mode.to_string()),
..Default::default()
};
assert_eq!(args.get_mode().as_deref(), Ok(mode));
}

// A present but unparseable string is not "not declared".
for mode in ["optiq", "gptq", "", " ", "Affine"] {
let args = QuantizationArgs {
mode: Some(mode.to_string()),
..Default::default()
};
let err = args
.get_mode()
.expect_err("a mode MLX cannot parse must not be handed out");
assert!(err.contains("mode"), "unhelpful error: {err}");
}
}
}
106 changes: 89 additions & 17 deletions src/models/gemma4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3738,7 +3738,14 @@ pub(crate) fn slice_layer_input(
/// Quantization schemes mlxcel can actually dequantize: MLX-native affine plus
/// the block-float families. Anything else uses a packing mlxcel does not
/// implement.
const SUPPORTED_QUANT_SCHEMES: &[&str] = &["affine", "mxfp4", "nvfp4", "mxfp8"];
///
/// Borrowed from the core rather than re-listed here (issue #973). This used to
/// be a private copy of the same four values, which is one list too many: the
/// core one is what every loader enforces, so a private copy could only ever
/// drift into accepting a mode the loaders reject or the reverse. `quant_method`
/// is checked against it too, since a `quant_method` naming an MLX-native scheme
/// is the same statement as a `mode` naming it.
const SUPPORTED_QUANT_SCHEMES: [&str; 4] = mlxcel_core::layers::SUPPORTED_QUANTIZATION_MODES;

/// Longest text-only prefill that still builds the dense `[l, l + offset]`
/// f32 prefill masks. Above this, the mask pair is left `None` and `attend`
Expand All @@ -3765,6 +3772,12 @@ const DENSE_PREFILL_MASK_MAX_TOKENS: i32 = 4096;
/// Inspects the top-level and `text_config`-nested `quantization` /
/// `quantization_config` objects. Pure over the parsed config JSON so the
/// policy is unit-testable without a model on disk.
///
/// The `mode` half of this is no longer gemma4-shaped and delegates to
/// [`mlxcel_core::layers::validate_quantization_mode`] (issue #973), which every
/// quantizing family now enforces at load. What stays here is the
/// `quant_method` policy: the external-format rejection this function was added
/// for and the ModelOpt NVFP4 exception that the gemma4 sanitize layer repacks.
pub(crate) fn validate_quantization_scheme(config: &serde_json::Value) -> Result<(), String> {
fn is_modelopt_nvfp4(obj: &serde_json::Value) -> bool {
let method = obj
Expand Down Expand Up @@ -3805,25 +3818,44 @@ pub(crate) fn validate_quantization_scheme(config: &serde_json::Value) -> Result
})
}

fn unsupported(raw: &str, location: &str, key: &str) -> String {
format!(
"Unsupported quantization scheme '{raw}' declared at {location}.{key}. \
mlxcel supports MLX-native affine and block-float (mxfp4 / nvfp4 / mxfp8) \
quantization only; external formats such as OptiQ / AWQ / GPTQ use a \
different packing that would dequantize to degenerate output. Re-export the \
model to an MLX affine quantization (mlx_lm.convert / mlx-vlm) before serving."
)
}

fn check_obj(obj: &serde_json::Value, location: &str) -> Result<(), String> {
for key in ["quant_method", "mode"] {
let Some(raw) = obj.get(key).and_then(|v| v.as_str()) else {
continue;
};
// `mode` delegates to the core allowlist so this guard and the loaders
// cannot disagree about the same config (issue #973). The comparison
// there is exact, which is deliberately stricter than the normalized one
// still applied to `quant_method` below: `mode` is a string MLX itself
// parses byte-for-byte, so accepting `" Affine"` here would only mean
// storing and forwarding a spelling the kernel then throws on. An empty
// `mode` is rejected on the same grounds; "not declared" is the absent
// key, which every config reader resolves to `"affine"` on its own.
if let Some(raw) = obj.get("mode").and_then(|v| v.as_str())
&& mlxcel_core::layers::validate_quantization_mode(raw).is_err()
{
return Err(unsupported(raw, location, "mode"));
}

// `quant_method` is a gemma4-shaped policy rather than a mode: it is the
// #467 external-format guard (OptiQ / AWQ / GPTQ) plus the ModelOpt
// NVFP4 exception, and it never reaches an MLX kernel, so it keeps the
// permissive normalization it has always had. The accepted set is still
// the core one, so the two cannot name different schemes.
if let Some(raw) = obj.get("quant_method").and_then(|v| v.as_str()) {
let norm = raw.trim().to_ascii_lowercase();
if key == "quant_method" && norm == "modelopt" && is_modelopt_nvfp4(obj) {
continue;
let accepted = norm.is_empty()
|| SUPPORTED_QUANT_SCHEMES.contains(&norm.as_str())
|| (norm == "modelopt" && is_modelopt_nvfp4(obj));
if !accepted {
return Err(unsupported(raw, location, "quant_method"));
}
if norm.is_empty() || SUPPORTED_QUANT_SCHEMES.contains(&norm.as_str()) {
continue;
}
return Err(format!(
"Unsupported quantization scheme '{raw}' declared at {location}.{key}. \
mlxcel supports MLX-native affine and block-float (mxfp4 / nvfp4 / mxfp8) \
quantization only; external formats such as OptiQ / AWQ / GPTQ use a \
different packing that would dequantize to degenerate output. Re-export the \
model to an MLX affine quantization (mlx_lm.convert / mlx-vlm) before serving."
));
}
Ok(())
}
Expand Down Expand Up @@ -5942,6 +5974,46 @@ mod quant_scheme_tests {
assert!(validate_quantization_scheme(&cfg).is_err());
}

/// The `mode` list is no longer owned here: it delegates to
/// `mlxcel_core::layers::validate_quantization_mode`, so this guard and the
/// loaders cannot disagree about the same config (issue #973). Delegation
/// also makes the comparison exact, which is stricter than the normalized
/// one this used to apply: a mode is a string MLX parses byte-for-byte, so
/// accepting a trimmed or case-folded spelling here would only mean storing
/// and forwarding a value the kernel then throws on.
#[test]
fn mode_check_delegates_to_the_core_allowlist() {
for mode in mlxcel_core::layers::SUPPORTED_QUANTIZATION_MODES {
let cfg = json!({ "quantization": { "group_size": 64, "bits": 4, "mode": mode } });
assert!(
validate_quantization_scheme(&cfg).is_ok(),
"MLX accepts {mode}, so this guard must too"
);
}

// Exact comparison: whitespace, casing and an empty string are all
// values MLX would throw on.
for mode in ["Affine", "AFFINE", " mxfp4", "mxfp4 ", "", " "] {
let cfg = json!({ "quantization": { "group_size": 64, "bits": 4, "mode": mode } });
let err = validate_quantization_scheme(&cfg)
.expect_err("a spelling MLX cannot parse must be rejected");
assert!(
err.contains("Unsupported quantization scheme"),
"actionable message: {err}"
);
}

// `quant_method` is a separate, gemma4-shaped policy (issue #467) and
// keeps its permissive normalization: it never reaches an MLX kernel.
for method in ["AFFINE", " affine ", ""] {
let cfg = json!({ "quantization": { "quant_method": method } });
assert!(
validate_quantization_scheme(&cfg).is_ok(),
"quant_method {method:?} names an MLX-native scheme and must stay accepted"
);
}
}

#[test]
fn rejects_awq_gptq_quant_method() {
for method in ["awq", "gptq", "AWQ"] {
Expand Down
Loading