fix(orchestrator): gate diffusion behaviour on registry kind, not the xdit name - #1117
Open
sushildubey171 wants to merge 1 commit into
Open
fix(orchestrator): gate diffusion behaviour on registry kind, not the xdit name#1117sushildubey171 wants to merge 1 commit into
sushildubey171 wants to merge 1 commit into
Conversation
… xdit name
``hunyuan_image3`` is registered ``kind=scriptable`` with
``throughput_unit="img/s"`` and ships baseline/profile configs, but several
call sites decide "is this a diffusion workload?" by comparing the framework
name to ``"xdit"``. Those sites silently treat it as an LLM workload.
Concretely, before this change a hunyuan_image3 session:
- reported ``tok/s`` from ``SharedState._roofline_throughput_unit`` and from
the bypass analysis report, contradicting its own registry entry
- got no workload-level diffusion roofline sidecar from the bypass route
- did not get the scriptable steady-state default, and used the LLM repeat
threshold when locating its denoise loop
- fell through ``compute_roofline_breakdown_from_state`` to the LLM decode
roofline instead of the diffusion ceiling
All of these now consult ``framework_registry`` (``is_scriptable`` /
``throughput_unit``), which is already the canonical predicate elsewhere in
the tree -- ``executors/profile.py`` calls ``is_scriptable()`` for the
server-less branch and only then narrows to ``xdit`` for the xfuser-specific
check, which is the pattern followed here.
The kernel tools keep working standalone: they try the registry and fall back
to a name check, mirroring the existing ``_is_scriptable_framework`` helper in
``tracelens_analysis``.
Deliberately NOT changed, because the ``xdit`` literal is correct there: the
xfuser pip name and xDiT git URL, ``XditAdapter`` registration, per-framework
config paths, the PATH guard for the ``xdit`` console script, the XDIT_* env
blacklist, the xDiT default grid seeds, and the xfuser baked-profiler check.
The two health-warning codes keep their ``bypass_xdit_*`` spelling for wire
stability even though the condition is now generic; only their messages are
reworded from "xDiT" to "diffusion". Renaming them is a separate call.
Behaviour for serving frameworks and for unknown/empty names is unchanged --
the registry falls back to sglang, so those still resolve to tok/s and the
decode roofline.
No test in the tree referenced hunyuan_image3 before this change, which is
why the divergence went unnoticed; the added tests assert against every
registry entry rather than a hardcoded list, so a future framework cannot
regress the same way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Description: what and why
hunyuan_image3is registeredkind=scriptablewiththroughput_unit="img/s"and shipsbaseline_hunyuan_image3.yaml/profile_hunyuan_image3.yaml— but several call sites decide "is this a diffusion workload?" by comparing the framework name to"xdit". Those sites silently treat it as an LLM workload.Before this change a
hunyuan_image3session:SharedState._roofline_throughput_unittok/simg/s(its own registry entry)tok/simg/scompute_roofline_breakdown_from_stateAll of these now consult
framework_registry(is_scriptable/throughput_unit), which is already the canonical predicate in 26 other places.executors/profile.py:632is the pattern I followed — it callsis_scriptable()for the server-less branch and only then narrows toxditfor the xfuser-specific check, with a comment noting hunyuan_image3 must not trigger it.The kernel tools keep working standalone: they try the registry and fall back to a name check, mirroring the existing
_is_scriptable_frameworkhelper intracelens_analysis.py.Deliberately not changed. I went through every
"xdit"literal in non-test code; most are correct and are left alone: thexfuserpip name and xDiT git URL,XditAdapterregistration, per-framework config paths, the PATH guard for thexditconsole script +hipcc, the XDIT_* env blacklist, the xDiT default grid seeds, and the xfuser baked-profiler verification.cli/model_gate.pyalready callsis_scriptable()with the literal only as an import-failure fallback.The two health-warning codes keep their
bypass_xdit_*spelling for wire stability even though the condition is now generic; only their messages change from "xDiT" to "diffusion". Renaming those codes felt like a separate call for you to make.Linked issue(s)
Addresses item 3 of #1114 (see my correction comment there — my original "15 hardcoded comparisons" was a raw grep count and overstated it; this PR covers the subset that is genuinely wrong). Does not close #1114, whose main ask is an SGLang diffusion route.
This is also the prerequisite that issue names: with the kind checks registry-driven, adding a new scriptable framework gets closer to the "single-table edit"
framework_registry's own docstring promises. I deliberately did not add ansglang-diffusionentry here — without a matching Magpie wrapper script (external repo) it would be dead config, and #1114 is asking you how you'd want that shaped.Tests: added/updated? commands run?
No test in the tree referenced
hunyuan_image3before this change, which is why the divergence went unnoticed. The new tests assert against every registry entry rather than a hardcoded list, so a future framework cannot regress the same way:FRAMEWORKS[*].kind/.throughput_unitfor all entries, in both the tool and the readercompute_roofline_breakdown_from_stateroutes xdit and hunyuan_image3 to the diffusion ceiling, and sglang/vllm/atom to the decode pathtok/s+ decodeI did not run the full suite to completion locally — it exceeds ~10 min in my environment — so I ran the kernel-agent tree plus the modules consuming these call paths.
Breaking changes: no
Serving frameworks and unknown/empty names are unchanged: the registry falls back to
sglang, so they still resolve totok/sand the decode roofline. The only behaviour that changes is forkind=scriptableframeworks other thanxdit— i.e.hunyuan_image3— which now gets the diffusion treatment its registry entry already declared. Warning code strings are unchanged.