Repository navigation
refactor(loading): remove dead is_special_weight_model_type wrapper - #1587
Merged
inureyes merged 1 commit intoSep 2, 2026
Merged
Conversation
`src/loading/special.rs` carried a pass-through wrapper around `model_metadata::is_special_weight_model_type` that had no callers and survived only behind `#[allow(dead_code)]`. The one test that exercises the predicate, `src/model_metadata_tests.rs`, imports the `model_metadata` original directly, so nothing reached the `loading::special` copy. Delete the wrapper and its `#[allow(dead_code)]`. The `use crate::model_metadata` import stays because `adapter_loading_unsupported_message` still goes through it. Validated with `cargo fmt --all -- --check`, `cargo check --workspace --all-targets --features metal,accelerate` and `cargo test --features metal,accelerate -p mlxcel --lib model_metadata_tests` (7 passed), all on a CPU-only MLX build (`MLXCEL_BUILD_METAL=OFF`). Closes lablup#1225
This was referenced Sep 2, 2026
inureyes
added
good first issue
inureyes
pushed a commit
that referenced
this pull request
Sep 2, 2026
## Summary
Replaces the fourteen copy-pasted `image::open(path).map_err(...)` closures in `generate_vlm.rs` and `generate_diffusion.rs` with a single `open_image` helper. The error text is byte-identical to the previous majority form; the drifted variants (inline `{error}` capture, bare `anyhow!` import, mixed `|e|`/`|error|` binders) disappear.
## Related issues
Closes #1245
## Type of change
- [x] `refactor`: internal restructuring without behavior change
## Test plan
- [x] `cargo fmt --all -- --check`
- [x] `cargo check --workspace --all-targets --features metal,accelerate` (initially on a CPU-only MLX build via `MLXCEL_BUILD_METAL=OFF`; superseded by the Metal gate below)
- [x] `cargo clippy -p mlxcel --bin mlxcel --features metal,accelerate -- -D warnings` (clean; in particular no `redundant_closure` on the `.map(|path| open_image(path))` sites, since the closure performs the `&PathBuf` to `&Path` coercion)
- [x] `RUST_MIN_STACK=16777216 cargo test --features metal,accelerate -p mlxcel --bin mlxcel commands::` (169 passed)
- [x] `grep -rn "Failed to load image" src/commands/` matches exactly one line, the helper body
- [ ] Real-checkpoint validation not run: the change does not touch inference, only the error path when `image::open` fails, and the success path returns the same `DynamicImage` as before
- [x] Full local gate from CONTRIBUTING.md on a Metal MLX build, run on a local branch that merges all eight re-implementation PRs (#1583, #1584, #1585, #1586, #1587, #1589, #1590, #1591) onto `main`: `cargo clippy --workspace --all-targets --features metal,accelerate -- -D warnings` (clean) and `cargo test --workspace --profile test-fast --features metal,accelerate --no-fail-fast -- --test-threads=1` (114 test binaries, 10385 passed, 0 failed, 326 ignored)
## Notes for reviewers
This re-implements the closed PR #1252 against current `main`. The issue counted eleven sites; `main` has since grown to thirteen in `generate_vlm.rs` plus the one in `generate_diffusion.rs`, all converted here. The helper lives in `generate_vlm.rs` next to its heaviest users and is `pub(crate)` so `generate_diffusion.rs` can reach it as `super::generate_vlm::open_image`; a separate `commands/util.rs` felt heavy for a one-function module, but it is a trivial move if you would rather have it there.
inureyes
pushed a commit
that referenced
this pull request
Sep 2, 2026
## Summary Removes the internal `(A4)` and `(B11)` task codes from doc comments and inline comments so the rendered rustdoc reads as plain sentences. `A4` was the `--surgery` CLI wiring and `B11` the llama-server `--cache-type-k/--cache-type-v` split flags; the commits that introduced them had their issue references scrubbed, so there is no public `(issue #NNN)` to substitute and the prefixes are simply dropped. ## Related issues Closes #1234 ## Type of change - [x] `docs`: documentation only ## Test plan - [x] `cargo fmt --all -- --check` - [x] `cargo check --workspace --all-targets --features metal,accelerate` (initially on a CPU-only MLX build via `MLXCEL_BUILD_METAL=OFF`; superseded by the Metal gate below) - [x] `grep -rnE '\((: )?(A4|B11)\)|\((A4|B11):' src/` returns nothing - [x] Full local gate from CONTRIBUTING.md on a Metal MLX build, run on a local branch that merges all eight re-implementation PRs (#1583, #1584, #1585, #1586, #1587, #1589, #1590, #1591) onto `main`: `cargo clippy --workspace --all-targets --features metal,accelerate -- -D warnings` (clean) and `cargo test --workspace --profile test-fast --features metal,accelerate --no-fail-fast -- --test-threads=1` (114 test binaries, 10385 passed, 0 failed, 326 ignored) ## Notes for reviewers This re-implements the closed PR #1240 against current `main`. The issue listed six sites; a full grep found 23 parenthesized occurrences across 17 files (`src/server`, `src/bin/mlx_server.rs`, `src/commands/serve.rs`, `src/cli/turbo_args.rs`, `src/loading`, `src/models`, `mlxcel-surgery`), all handled here. Bare narrative mentions such as "once A4's `--surgery` flag lands" in the `mlxcel-surgery` integration-test module docs are left alone: they are stale in a different way (the flag has landed) and deserve their own rewrite rather than a mechanical prefix drop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Deletes the dead
is_special_weight_model_typepass-through wrapper insrc/loading/special.rs, together with the#[allow(dead_code)]that kept it compiling. The only consumer of the predicate,src/model_metadata_tests.rs, already calls themodel_metadataoriginal.Related issues
Closes #1225
Type of change
refactor— internal restructuring without behavior changeTest plan
cargo fmt --all -- --checkcargo check --workspace --all-targets --features metal,accelerate(initially on a CPU-only MLX build viaMLXCEL_BUILD_METAL=OFF; superseded by the Metal gate below)cargo test --features metal,accelerate -p mlxcel --lib model_metadata_tests(7 passed)grep -rn is_special_weight_model_type src/now matches onlysrc/model_metadata.rsandsrc/model_metadata_tests.rsmain:cargo clippy --workspace --all-targets --features metal,accelerate -- -D warnings(clean) andcargo test --workspace --profile test-fast --features metal,accelerate --no-fail-fast -- --test-threads=1(114 test binaries, 10385 passed, 0 failed, 326 ignored)Notes for reviewers
This re-implements the closed PR #1237 against current
main. Not an inference change, so no real-checkpoint validation applies.