Repository navigation
test(vision): add qwen2.5-vl-3b-4bit warmup regression guard - #38
Merged
Merged
Conversation
`qwen2.5-vl-3b-4bit` was marked FAIL:warmup in the M5 Max benchmark sweep. The failure surfaced in the same-process bench harness: a warmup pass left stale generator-owned KV state, and the subsequent measured pass broadcast a 4-D VLM attention mask against those stale keys and aborted. PR #34 (`fix: reset model-owned VLM fallback state`) fixed this transitively by rebuilding generator-owned KV caches before each single-row run in `CxxGenerator::reset_with_model()`; `qwen2_5_vl` carries no model-owned fallback state, so no additional production code change is needed here. This commit adds the regression test `qwen2_5_vl_single_row_bench_prefill_succeeds_with_m5_warmup` in `tests/vlm_concurrency.rs`, which drives the exact FAIL:warmup repro path — an image VLM run with `--warmup-tokens 20` so the harness performs both a warmup pass and a measured pass in the same process. It mirrors the existing `gemma3n_vlm_single_row_bench_prefill_accepts_m5_tile_padding` guard and is `#[ignore]`d behind local Qwen2.5-VL weights and the `mlxcel-bench-decode` binary. The M5 Max benchmark doc (`docs/benchmark_results/model_tests_m5max.md`) is updated to reflect the now-passing run: text prefill 1529.43 tok/s / decode 165.11 tok/s (EOS at 39 tokens) and VLM prefill 1696.53 tok/s / decode 156.83 tok/s (EOS at 22 tokens, 1.61x vs M1 Ultra). The pass count rises from 91 to 92, the fail count drops from 5 to 4, qwen2.5-vl-3b-4bit leaves both the Known Issues table and the mlx-lm unsupported-architecture list, and it gains a real mlx-lm decode comparison row (156.83 vs 98.53 tok/s, 159%).
inureyes
added
type:test
7 tasks
sjswerdloff
added a commit
to sjswerdloff/mlxcel
that referenced
this pull request
Jul 12, 2026
…tion witnessed), Clement crossing, wiring = far-side pickup, lablup#38 filed Co-Authored-By: violet-14057653 <violet@sjstargetedsolutions.co.nz> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
be-free-snowboy
pushed a commit
to freestylejs/snowbrain-mlxcel
that referenced
this pull request
Oct 27, 2026
`qwen2.5-vl-3b-4bit` was marked FAIL:warmup in the M5 Max benchmark sweep. The failure surfaced in the same-process bench harness: a warmup pass left stale generator-owned KV state, and the subsequent measured pass broadcast a 4-D VLM attention mask against those stale keys and aborted. PR lablup#34 (`fix: reset model-owned VLM fallback state`) fixed this transitively by rebuilding generator-owned KV caches before each single-row run in `CxxGenerator::reset_with_model()`; `qwen2_5_vl` carries no model-owned fallback state, so no additional production code change is needed here. This commit adds the regression test `qwen2_5_vl_single_row_bench_prefill_succeeds_with_m5_warmup` in `tests/vlm_concurrency.rs`, which drives the exact FAIL:warmup repro path — an image VLM run with `--warmup-tokens 20` so the harness performs both a warmup pass and a measured pass in the same process. It mirrors the existing `gemma3n_vlm_single_row_bench_prefill_accepts_m5_tile_padding` guard and is `#[ignore]`d behind local Qwen2.5-VL weights and the `mlxcel-bench-decode` binary. The M5 Max benchmark doc (`docs/benchmark_results/model_tests_m5max.md`) is updated to reflect the now-passing run: text prefill 1529.43 tok/s / decode 165.11 tok/s (EOS at 39 tokens) and VLM prefill 1696.53 tok/s / decode 156.83 tok/s (EOS at 22 tokens, 1.61x vs M1 Ultra). The pass count rises from 91 to 92, the fail count drops from 5 to 4, qwen2.5-vl-3b-4bit leaves both the Known Issues table and the mlx-lm unsupported-architecture list, and it gains a real mlx-lm decode comparison row (156.83 vs 98.53 tok/s, 159%).
inureyes
added a commit
that referenced
this pull request
Oct 30, 2026
## Confirmed mechanism The abort is a `ModelProvider` worker thread freeing its model after `main` has returned. Reproduced on this box (kernel 7.0.0, same as the failing runner image) with the CI configuration (debug profile, no `cuda` feature, `MLXCEL_CI_PP_MODEL=qwen3-0.6b-4bit`); gdb with a breakpoint on `__cxa_pure_virtual` caught it on 3 of 3 attempts. Aborting thread (frames condensed): ``` #0 __cxa_pure_virtual () #1 mlx::core::allocator::Data::~Data at mlx/allocator.h:79 (allocator().free(buffer)) #11 mlx::core::array::ArrayDesc::~ArrayDesc at mlx/array.cpp:333 #22 mlx_cxx::MlxArray::~MlxArray at cpp/mlx_cxx_bridge.h:27 #38 drop_glue<mlxcel_core::cache::paged::PagedBlockPool> #49 spawn_model_worker_with_batch_config::{closure#0}::{closure#0} at src/server/model_worker.rs:893 #55 mlxcel::worker_failfast::run_core_thread_or_abort ``` Main thread at the same instant: ``` #6 blas_thread_shutdown_ / #8 gotoblas_quit (libopenblas finalizer) #9 _dl_call_fini / #10 _dl_fini #11 __run_exit_handlers / #12 exit ``` The worker (still joined by nobody: `Drop for ModelProvider` only sends `ModelRequest::Shutdown`, and its `JoinHandle` sits in the `observe_worker_exit` watcher thread, which gdb shows blocked in `pthread_join` on it) is dropping the scheduler's `PagedBlockPool`. Each array free goes through `allocator().free()`. On the CPU backend `allocator()` is `static CommonAllocator allocator_` in `mlx/backend/no_gpu/allocator.cpp`, a function-local static whose destructor runs from `__run_exit_handlers`; after it, the object's vtable is the abstract `Allocator`'s and `free` is pure virtual. The Metal, CUDA and ROCm allocators are heap-leaked on purpose and never reach this state, which is why only the CPU CI build aborts. The kernel change in the runner image only altered the scheduling that exposes the window; the test never joined the two providers. ## Fix - `ModelProvider::shutdown_and_wait(timeout)` (`src/server/model_provider.rs`): sends the shutdown signal, waits on the existing `WorkerExitObserver`, returns `Err("server model worker did not exit within 120s")` on expiry (the duration is printed with `Debug`, so the production text is unchanged and a sub-second test timeout reads `500ms`, not `0s`) and `Err("server model worker panicked: ...")` on a panic. Idempotent; an exited worker and a closed channel return `Ok`. `WORKER_EXIT_TIMEOUT` (120 s) moves here as `pub` and is re-exported from `mlxcel::server` so the integration test can use it. `InProcessServer::shutdown` delegates to it. `Drop` stays non-blocking. - `tests/pipeline_ci_multi_stage_real_models.rs`: the shared helper calls `shutdown_and_wait(WORKER_EXIT_TIMEOUT)` on the coordinator provider, then the dense one, and `expect`s both before shutting the stage handles down. `tests/pipeline_server_remote_real_models.rs` now does the same instead of dropping its two providers (CUDA and Metal only today, where the allocators are leaked, but exposed on a CPU runner). - Unit tests in `src/server/model_provider_tests.rs`: cooperative worker (`Ok`, observed, second call `Ok`), already-exited worker (`Ok` with a zero timeout), worker panicking during teardown (`Err` carrying the message), worker ignoring shutdown (`Err` after a 1 s timeout, then `Ok` once released). ## Production exposure (per path) - `mlxcel-server` single-model mode (`start_server`): affected. `serve_http` returning (bind or TLS error) and the five side-model resolution errors after the chat worker started loading all returned with the worker alive. Fixed: everything after the provider is created moved into the function `serve_with_model_provider`, whose result flows through `join_model_worker_before_exit`. That logs the serve error at error level first (`server stopped: ...; waiting for the model worker to exit`), so a bind or TLS failure during a long load is not a silent wait, then calls `shutdown_and_wait` on the blocking pool and logs `model worker exited before process exit`. The serve error is returned unchanged for `main` to print, and a shutdown failure never replaces it. SIGINT/SIGTERM in this mode still die by default disposition (no exit handlers run), unchanged. The audio, embedding, image and rerank side workers are trait objects without an exit observer and are not joined here; the same hazard class applies to them on the CPU backend and is tracked in #2745 (with the sleeping-worker reload on shutdown, the uncancellable load past the join timeout, and the `--pp-peer` signal path). - Router mode (`run_router_server`): partly affected. The Ctrl-C arm already drained through `shutdown_all`, whose `unload` waits on each worker's exit observer (and the entry operation guard makes it wait for an in-flight `build_model_app` too). The `serve_http` error arm returned without draining, so a bind failure after `load-on-startup` exited under loading workers. Fixed: both arms drain before returning. The listener error is logged before the drain starts, and a Ctrl-C handler that cannot be installed now drains before its error is returned instead of skipping `shutdown_all`. Residual: a worker that ignores shutdown past `ROUTER_SHUTDOWN_TIMEOUT` (150 s) still exits with handlers running; the alternative is hanging. - `--pp-peer` stage (`serve_remote_pipeline_stage`): not affected. `RemoteStageServiceHandle::shutdown` joins the stage thread, which owns the stage model and its current-thread runtime. - `mlxcel run` / `chat`: not affected. They already join through `InProcessServer::shutdown` (#2224), which now shares the implementation. `tests/server_exit_joins_worker.rs` drives a real `mlxcel-server` into the bind-failure exit (the test holds the port) for the single-model and the router `load-on-startup` paths and asserts exit code 1, no signal, no C++ abort text, and the join log line. The router test matches `router: lifecycle shutdown completed attempted=1 completed=1`, since a failed `begin_load` prints the same message with `attempted=0`. Both skip without the `qwen3-0.6b-4bit` checkpoint. ## Before / after Command, on the debug CPU binary exactly as CI runs it: ``` BIN=target/debug/deps/pipeline_ci_multi_stage_real_models-<hash> for i in $(seq 50); do MLXCEL_CI_PP_MODEL=qwen3-0.6b-4bit "$BIN" pipeline_multi_stage_two_host_logical_smoke --ignored --nocapture --test-threads=1; done ``` - Before the fix (origin/main 8385f40): `aborts: 44/50` (every abort log carries `pure virtual method called`; the other 6 runs exited cleanly) - After the fix: `aborts: 0/50` (same binary rebuilt from this branch, same box, loops overlapped in time) ## Test plan - [x] `cargo test --profile test-fast --features cuda -p mlxcel --lib server::model_provider:: -- --test-threads=1` - [x] `cargo test --profile test-fast --features cuda -p mlxcel --lib server::in_process:: -- --test-threads=1` - [x] `tests/server_exit_joins_worker.rs` (both tests, real checkpoint, `MLXCEL_REQUIRE_MODELS=1`) - [x] the smoke test on the debug CPU binary, 50 consecutive runs - [x] `cargo check --profile test-fast --features cuda,test-utils -p mlxcel --test pipeline_server_remote_real_models` - [x] `cargo clippy --profile test-fast --features cuda -p mlxcel --lib --tests -- -D warnings`, `cargo fmt --all -- --check` ## CI stability Ten consecutive `workflow_dispatch` runs of `pipeline-parallel-ci.yml` on this branch at head `03c1e365`, dispatched one at a time (the concurrency group cancels overlapping runs on the same ref). All ten passed, and all ten ran on `ubuntu24/20261025.353`, the runner image every one of the original aborts ran on. No job log contains `pure virtual method called`. | # | Run | Head | Result | Runner image | |---|---|---|---|---| | 1 | [41045436759](https://github.com/lablup/mlxcel/actions/runs/41045436759) | `03c1e365` | success | `ubuntu24/20261025.353` | | 2 | [41048002710](https://github.com/lablup/mlxcel/actions/runs/41048002710) | `03c1e365` | success | `ubuntu24/20261025.353` | | 3 | [41049748107](https://github.com/lablup/mlxcel/actions/runs/41049748107) | `03c1e365` | success | `ubuntu24/20261025.353` | | 4 | [41051499873](https://github.com/lablup/mlxcel/actions/runs/41051499873) | `03c1e365` | success | `ubuntu24/20261025.353` | | 5 | [41053271228](https://github.com/lablup/mlxcel/actions/runs/41053271228) | `03c1e365` | success | `ubuntu24/20261025.353` | | 6 | [41055089949](https://github.com/lablup/mlxcel/actions/runs/41055089949) | `03c1e365` | success | `ubuntu24/20261025.353` | | 7 | [41056817489](https://github.com/lablup/mlxcel/actions/runs/41056817489) | `03c1e365` | success | `ubuntu24/20261025.353` | | 8 | [41058570988](https://github.com/lablup/mlxcel/actions/runs/41058570988) | `03c1e365` | success | `ubuntu24/20261025.353` | | 9 | [41060493226](https://github.com/lablup/mlxcel/actions/runs/41060493226) | `03c1e365` | success | `ubuntu24/20261025.353` | | 10 | [41062232317](https://github.com/lablup/mlxcel/actions/runs/41062232317) | `03c1e365` | success | `ubuntu24/20261025.353` | The two `pull_request` runs of the same workflow on this PR also passed on `ubuntu24/20261025.353`: [41024288311](https://github.com/lablup/mlxcel/actions/runs/41024288311) at `e4312b89` and [41045135070](https://github.com/lablup/mlxcel/actions/runs/41045135070) at `03c1e365`. Closes #2738
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
Adds the
qwen2.5-vl-3b-4bitwarmup regression guard.qwen2.5-vl-3b-4bitwas failing warmup in the M5 Max bench sweep: the same-process harness ran a warmup pass that left stale generator-owned KV state, and the measured pass then broadcast a 4-D VLM attention mask against those stale keys and aborted. PR #34 (fix: reset model-owned VLM fallback state) already fixed the root cause by rebuilding generator-owned KV caches inCxxGenerator::reset_with_model(), andqwen2_5_vlhas no model-owned fallback state, so this change adds only the guarding test plus the benchmark-doc update — no production code change.Changes
tests/vlm_concurrency.rs: new#[ignore]d testqwen2_5_vl_single_row_bench_prefill_succeeds_with_m5_warmupthat exercises the exact FAIL:warmup repro path (image VLM with--warmup-tokens 20, so the harness performs both a warmup pass and a measured pass in the same process), mirroring the existinggemma3n_vlm_single_row_bench_prefill_accepts_m5_tile_paddingguard.docs/benchmark_results/model_tests_m5max.md: qwen2.5-vl-3b-4bit flipped from FAIL to passing (text prefill 1529.43 / decode 165.11 tok/s; VLM prefill 1696.53 / decode 156.83 tok/s, 1.61x vs M1 Ultra); pass count 91 → 92, fail count 5 → 4; row removed from the Known Issues table and the mlx-lm unsupported-architecture list, and given a real mlx-lm decode comparison (156.83 vs 98.53 tok/s, 159%).Testing
cargo fmt --all -- --check— clean.cargo clippy --test vlm_concurrency --features metal,accelerate -- -D warnings— clean.#[ignore], needs local Qwen2.5-VL weights and themlxcel-bench-decodebinary), matching the other VLM single-row guards, so it is not exercised by CI.