Skip to content

[Bugfix] Fix NameError in OmniDiffusion init for Bagel/NextStep after GLM-Image merge#1563

Closed
lishunyang12 wants to merge 1 commit intovllm-project:mainfrom
lishunyang12:fix/bagel-nameerror-after-glm-image
Closed

[Bugfix] Fix NameError in OmniDiffusion init for Bagel/NextStep after GLM-Image merge#1563
lishunyang12 wants to merge 1 commit intovllm-project:mainfrom
lishunyang12:fix/bagel-nameerror-after-glm-image

Conversation

@lishunyang12
Copy link
Contributor

@lishunyang12 lishunyang12 commented Feb 28, 2026

Summary

  • PR [Perf] GLM Image #920 (GLM-Image support) refactored the model-type dispatch in OmniDiffusion.__init__ but introduced a bug: the GLM-Image and single-architecture branches assign to a local pipeline_class variable, while lines 93-98 unconditionally read it. When Bagel or NextStep models hit their branches (which set od_config directly), pipeline_class is never defined, causing a NameError.
  • Fix: make each branch self-contained — every branch sets od_config.model_class_name, tf_model_config, and calls update_multimodal_support() directly. No shared pipeline_class variable needed.

Test plan

  • Verify Bagel model loads without crash: OmniDiffusion(model="ByteDance-Seed/BAGEL-7B-MoT")
  • Verify GLM-Image still works: OmniDiffusion(model="THUDM/GLM-Image-1.0")
  • Verify single-architecture fallback path still works

@lishunyang12
Copy link
Contributor Author

lishunyang12 commented Feb 28, 2026

Verified the bug and fix by simulating the dispatch logic in OmniDiffusion.__init__ :

--- BEFORE fix (main branch logic) ---
  Bagel       : FAIL: cannot access local variable 'pipeline_class' where it is not associated with a value
  NextStep    : FAIL: cannot access local variable 'pipeline_class' where it is not associated with a value
  GLM-Image   : OK
  Single-arch : OK

--- AFTER fix (PR #1563 logic) ---
  Bagel       : OK
  NextStep    : OK
  GLM-Image   : OK
  Single-arch : OK

Bagel/NextStep branches set od_config directly but fall through to lines 93-98 which read the never-defined pipeline_classNameError. Making each branch self-contained fixes it cleanly.

@hsliuustc0106
Copy link
Collaborator

Verified the bug and fix by simulating the dispatch logic in OmniDiffusion.__init__ :

--- BEFORE fix (main branch logic) ---
  Bagel       : FAIL: cannot access local variable 'pipeline_class' where it is not associated with a value
  NextStep    : FAIL: cannot access local variable 'pipeline_class' where it is not associated with a value
  GLM-Image   : OK
  Single-arch : OK

--- AFTER fix (PR #1563 logic) ---
  Bagel       : OK
  NextStep    : OK
  GLM-Image   : OK
  Single-arch : OK

Bagel/NextStep branches set od_config directly but fall through to lines 93-98 which read the never-defined pipeline_classNameError. Making each branch self-contained fixes it cleanly.

fix DCO

… GLM-Image merge

Signed-off-by: lishunyang <lishunyang12@163.com>
@lishunyang12 lishunyang12 force-pushed the fix/bagel-nameerror-after-glm-image branch from a4388be to 0f29d8e Compare February 28, 2026 04:23
@hsliuustc0106 hsliuustc0106 added the ready label to trigger buildkite CI label Feb 28, 2026
od_config.model_class_name = pipeline_class
od_config.tf_model_config = TransformerConfig()
od_config.update_multimodal_support()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, Shunyang, I thinke these three lines of code is common, which should not be deleted and added in former if-else code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants