Skip to content

bench: greedy Qwen3.5 cells and ITL in results.json - #909

Merged
Gaurav-Shah05 merged 2 commits into
mainfrom
users/atindra/b300-bench-llm
Oct 27, 2026
Merged

Gaurav-Shah05 merged 2 commits into
mainfrom
users/atindra/b300-bench-llm

Conversation

@merceod

@merceod merceod commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Two small runner fixes from the B300 text/VLM comparison.

  • Qwen3_5_Dense.get_model_kwargs sends temperature: 0 and chat_template_kwargs: {enable_thinking: true}. Without it mstar sampled at its 0.6 default while vLLM and SGLang ran at 1.0, and the 0.8B/2B chat templates rendered without the think block that mstar always opens. The three engines were not generating the same thing.
  • results.json now carries itl_s, text_token_throughput, total_text_tokens, mean_text_tokens and total_input_tokens at full precision next to ttft_s. Before, ITL and tok/s only existed in the rounded console summary, which is not enough to compare 2-3 ms ITLs across engines.

Used for every Qwen3.5 0.8B/2B/4B/9B cell against vLLM 0.32.0 and SGLang 0.5.22 on B300 (closed_loop, 512 in / 256 out, ignore-eos). No engine code touched.

…esults.json

qwen3.5 sent no sampling params, so mstar sampled at its 0.6 default. Also pass
enable_thinking through chat_template_kwargs so vllm/sglang render the 0.8b/2b
prompts the way mstar does. results.json now carries itl, tok/s and token counts
(the console table rounds to 1 ms).
@mstar-review

mstar-review Bot commented Oct 27, 2026

Copy link
Copy Markdown

Automated review

🟡 No findings on da469cd, light review

Description

  • The claim that vLLM and SGLang 'ran at 1.0' before this change doesn't match the code: VLLMOmni.send_request (benchmark/request.py:1069) and SGLangOmni.send_request (benchmark/request.py:1379) already hardcode temperature: 0.0 in the payload, so the new temperature only changes M*'s native path (OurSystem, which sends the model kwargs as JSON at request.py:730); please correct the rationale or say which client actually ran vLLM/SGLang at 1.0.
  • The description doesn't mention that Qwen3_8_Dense inherits from Qwen3_5_Dense, so Qwen3.8 benchmark runs also switch to greedy decoding with enable_thinking: true, and their results are no longer comparable to earlier Qwen3.8 numbers.
  • The description says what the change was used for, but not which checks were run on this commit (for example ruff check ., or one benchmark.runner invocation with the exact command showing the new results.json keys).
What the review checked
  • AggregateMetrics (benchmark/request.py:382-409) has itl, text_token_throughput, total_text_tokens, mean_text_tokens and total_input_tokens, and aggregate_metrics fills them in, so the new results.json keys serialize without errors; itl values are LatencyStats, which _stats already handles.
  • temperature: 0.0 reaches SamplingReqConfig through Qwen3_5Model.get_request_resource_configs (qwen3_5_model.py:817), and the sampler treats temperature 0 as greedy (sampler/utils.py:1320).
  • On M*'s native path the extra chat_template_kwargs key is harmless: Qwen3.5's process_prompt reads a flat enable_thinking and defaults it to True (qwen3_5_model.py:524). The OpenAI adapter unpacks chat_template_kwargs (api_server/openai/adapters.py:818), so OursOpenAI gets the same render.
  • In the vLLM and SGLang payloads, get_model_kwargs is merged after the hardcoded temperature, so the new values come through and don't conflict with it.

Advisory, not a merge gate. Rules: AGENTS.md. Reply to discuss.

@Gaurav-Shah05 Gaurav-Shah05 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.

LGTM

Comment thread benchmark/base.py Outdated
# Greedy everywhere (mstar's sampler defaults to 0.6), and the same prompt
# render: mstar always opens a <think> block, while the 0.8B/2B templates
# only do when asked, so vLLM/SGLang get it through chat_template_kwargs.
return {"temperature": 0.0, "chat_template_kwargs": {"enable_thinking": True}}

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.

M*'s Qwen3.5 reads a flat enable_thinking (default True) and ignores chat_template_kwargs, so its think block comes from that default, not from this.

Sending enable_thinking: True flat as well would keep the three renders the same if the default changes.

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.

fixed it in e82e4e9

mstar's Qwen3.5 reads a flat enable_thinking and ignores chat_template_kwargs, which only the Cosmos3-Edge adapters map, so flipping the chat_template_kwargs value alone would change vLLM and SGLang but not mstar. vLLM and SGLang ignore the flat key.
@Gaurav-Shah05
Gaurav-Shah05 merged commit b4909bc into main Oct 27, 2026
6 checks passed
@Gaurav-Shah05
Gaurav-Shah05 deleted the users/atindra/b300-bench-llm branch October 27, 2026 18:02
dghosef added a commit that referenced this pull request Oct 28, 2026
Main brought (since 6cfe39c): #892 Parakeet and Nemotron ASR with the
transducer as a graph loop (and the shared mrope_position_ids helper),
#917 pytest 9.2 flag proxies, #897 one captured all-to-all for Ulysses,
#915 NCCL graph registration off by default, #836 Qwen3-Omni Thinker
prefix caching (ARNodeSubmodule.cuts_tensor_inputs, marks_dynamic_dims,
the conductor's _streamed_partitions), #909 bench cells, #900 Command A+
batched image tiles.

Conflict:
- mstar/model/submodule_base.py: main's ARNodeSubmodule.cuts_tensor_inputs
  beside the stack's token_stop and its stop checks. Independent class
  attributes; both kept.

Ported onto the stack's mechanisms:
- mstar/model/parakeet/submodules.py: check_stop_batched drops the
  step_walk parameter (the stack's three-argument signature) and reads
  its "active" flags through HostRows.first_values instead of its own
  row-map loop. Its stop is a flag, not a token, so it has no TokenStop.
- test/modular/test_transducer_loop.py: the call without step_walk.

The conductor's _streamed_partitions (#836) sits beside _pick_replicas /
_register_request; it reads only partition_states and _resource_specs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dghosef added a commit that referenced this pull request Oct 28, 2026
Brings #858's merge of main: #892 Parakeet / Nemotron ASR, #917, #897
Ulysses all-to-all, #915, #836 Qwen3-Omni Thinker prefix caching, #909,
#900.

Conflicts:
- docs/adding_models.rst: main's marks_dynamic_dims sentence and #859's
  compile_regions paragraph are independent facts; both kept, in one
  paragraph.
- mstar/model/qwen3_omni/submodules.py (3 hunks, plus auto-merged hunks
  in the same class): AudioEncoderSubmodule. Two mechanisms for one cause
  (the HF encoder recompiling per clip length): #859 runs it eagerly
  (disable_torch_compile) and packs a step's clips into calls of at most
  MAX_BATCH_FRAMES; #836 builds the encoder's chunk tensors in
  prepare_inputs with mark_dynamic so the per-clip forward compiles once
  (marks_dynamic_dims, _max_seqlen). One mechanism: #859's class verbatim.
  Under disable_torch_compile the engine never compiles the encoder, so
  #836's concern (a recompiled graph rounding differently from the one
  whose output the prefix cache holds) cannot arise. The auto-merge had
  taken #836's kwargs-only forward beside #859's forward_batched, which
  calls forward with the clips positionally; that keep-both is undone.
  Main's VisionEncoderSubmodule (marks_dynamic_dims, prepared indices) and
  Thinker changes (cuts_tensor_inputs, split_inputs, mrope_position_ids,
  image_span) are kept; #859 did not change those. Imports: both
  cp_embed_accumulate and mrope_position_ids; the audio helpers only
  #836's prepare_inputs used are dropped.

Tests following from it (test/modular/test_qwen3_omni_prefix_cache.py):
- the test that #836's prepared chunk tensors embed a clip as the encoder
  does alone is deleted with that code;
- the compile test now asserts the audio encoder stays eager while its
  output is cached;
- the CUDA no-recompile test keeps its vision case only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dghosef added a commit to dghosef/mstar that referenced this pull request Oct 28, 2026
…igin/main 73c6c17) into merge/860-main

Brings main's mstar-project#892 Parakeet / Nemotron ASR, mstar-project#917, mstar-project#897 Ulysses as one
captured all-to-all (prefer_all_gather and the all-gather exchange gone,
Cosmos3 captures every tier under TP x SP), mstar-project#915, mstar-project#836 Qwen3-Omni Thinker
prefix caching, mstar-project#909 and mstar-project#900, with mstar-project#858's and mstar-project#859's resolutions.

No conflicts. Files both sides touched (docs/clients.rst,
docs/installation.rst, docs/models.rst, sequence_parallel.py,
cosmos3/submodules.py, test_accelerator_graph_capture.py,
test_flux2_klein_model.py): each merged change equals mstar-project#859's own delta
since 3166401, so nothing kept both sides. mstar-project#860's my_rows / gather_rows
(Wan2.2's CFG split) sit beside main's new ulysses_attention and use
CommGroup.all_gather, which mstar-project#897 left unchanged; nothing in mstar-project#860 passed
prefer_all_gather.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants