[Feat/Fix] Refactoring Llava models into single file - #475
Merged
Merged
Conversation
2. Fix conversation templates issue.
Contributor
Author
|
This is my test, after refactoring, llava_qwen and llava_llama3 works correctly. (please ignore the output of the template message, it's for debug and removed in commit.) 
|
Contributor
Author
|
@merrymercy @BabyChouSr @Qubitium Thanks for maintaining such wonderful project. Please check this PR and feel free to give more suggestions, thanks! |
Luodian
changed the title
timethink
pushed a commit
to timethink/sglang
that referenced
this pull request
Mar 9, 2025
vschandramourya
pushed a commit
to vschandramourya/sglang
that referenced
this pull request
Feb 3, 2026
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
EdwardXuy
pushed a commit
to shun8686/sglang
that referenced
this pull request
May 9, 2026
add minimax per testcases
efschu
added a commit
to efschu/htsglang
that referenced
this pull request
Aug 3, 2026
…ockstep max (two all-reduces per layer), barrier skew reported per candidate; zero-parameter skew prediction 27.6 vs 27.9 ms/1k measured; FP8 ladder byte-identical, INT8 honestly has no prefill lever on this rig; floor-explosion root = harness clock ramp (-> sgl-project#459/sgl-project#483)
efschu
added a commit
to efschu/htsglang
that referenced
this pull request
Aug 3, 2026
… correct the §1.3 model, and make the sgl-project#431 guard non-blocking Two commits: test(sgl-project#483,sgl-project#459): sgl-project#483 ("check_regressions prices prefill on stale rates") is REFUTED -- check_regressions already prices on the RESOLVED rates (landed with sgl-project#475). What was actually missing was the PIN: the only test calling the function is checkpoint-gated and skips in every hermetic run, so a revert would be invisible. test_check_regressions_pricing_483.py is the hermetic falsifier. Also makes the s12 harness honest: an absent measurement is None, never 0.0 (a spec-off tick has no accept length, and 0.0 silently survives every arithmetic it enters); and any procedural property a verdict rests on (warm-up discarded, draws back-to-back, gap between draws) is recorded as a measured number in the artifact, not taken on the harness's word -- the s12 warm-up draw ran for months and was discarded silently, so a warmed-up point and a cold one wrote the same file. perf(sgl-project#517): names the actual decode-loop BAR1 collectives and corrects the catalog's round model -- a decode round is NOT free of host-path collectives (the belief the old model rested on). Per NEXTN round there are 3 replay boundaries (the draft chain is one captured graph, not one per step) and 5 host-path BAR1 broadcasts, all sgl-project#50 speculative rank-agreement syncs, reaching barlink because a barlink boot never constructs pynccl. Measured cost: the sgl-project#431 blocking status-word read costs -9.22% decode TPS against the same-tree NCCL baseline; removing it recovers +2.68%. Fix: the abort-guard status word is now read asynchronously (non-blocking D2H + cudaEventQuery, returning what an earlier check staged), trading reporting LATENCY for zero stream synchronization per check -- detection is unaffected because ctlStatus is sticky. SGLANG_BARLINK_BAR1_ABORT_MAX_LAG (default 4) bounds the deferral with a can-fail proof; ..._ABORT_DEFER=0 restores the exact pre-sgl-project#517 blocking behavior. NOTE_517_bar1_guard_desk.md has the full model, desk-only (no GPU arm run yet -- ticketed). Two real conflicts, both resolved by combining rather than choosing a side: - FEATURE_CATALOG.md §12 (same insertion-point pattern as every merge today): HEAD's "Verified carve-outs (sgl-project#514)" paragraph kept, this branch's two sgl-project#459 paragraphs ("absent measurement is None" / "procedural property must be in the artifact") appended after. Two OTHER hunks (the sgl-project#483-REFUTED note near §2, the "Guard cost" note near §7) landed with no conflict at all -- neither region had been touched since this branch's base. - test/registered/unit/distributed/_bar1_marker_source.py: both HEAD (via sgl-project#514's earlier edits) and this branch carry DIFFERENT LINE_* pins into barlink.py/barlink_bar1.py, because each was written against its own version of those files. Neither side's numbers are correct once both sets of source edits are combined. Resolved by reading the ACTUAL post-merge source: grepped the real logger.info/logger.warning/raise RuntimeError call-start lines in the merged barlink.py and barlink_bar1.py (2236/2250/3207/810) and verified all four renderers execute correctly against them before committing -- not a textual merge of the two guesses. All 17 catalog sections and every other additive paragraph verified present. Tests (CUDA_VISIBLE_DEVICES=99, PYTHONPATH pinned to this worktree): - The five directly touched/new test files (_bar1_marker_source.py's renderers verified live against the real merged source, plus test_barlink_bar1_abort_431.py, test_barlink_bar1_abort_deferred_517.py, test_s12_harness_459.py, test_check_regressions_pricing_483.py): 67 + 7 passed, 0 failed. - test_s12_log_analyse.py specifically (s12_log_analyse.py is touched): same 4 pre-existing failures, unchanged (fixture-data-dependent, not source-code-dependent). - test/registered/unit/distributed/ (full): 24 failed, 2206 passed, 8 skipped, 723 subtests -- same 24 failing IDs as the established baseline, byte-identical. - ruff check --select=F401,F821,UP037: clean. - codespell: 7 hits, all confirmed pre-existing (a German prose sample string already in scripts/gpu_battery/s12_prefill_kurve.py and s14_decode_punkt.py at this branch's base, untouched by its diff).
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
2 participants
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.
This PR includes the addition of new model classes for handling different configurations.
Key Changes
Introduced
LlavaQwenForCausalLMandLlavaMistralForCausalLMclasses, extending theLlavaLlamaForCausalLMwith specific configurations forQwen2andMistralmodels respectively.Modified the
model_runner.pyto refactor the entry class definition to To support multiple model classes in one module (we make different llava with different llms into one module to clean the code).Fix
llava_qwenandllava_llama3model