Skip to content

Fix raise NotImplemented to raise NotImplementedError in wrap_llm_lora#1259

Open
Mr-Neutr0n wants to merge 1 commit intoOpenGVLab:mainfrom
Mr-Neutr0n:fix/raise-not-implemented-error
Open

Fix raise NotImplemented to raise NotImplementedError in wrap_llm_lora#1259
Mr-Neutr0n wants to merge 1 commit intoOpenGVLab:mainfrom
Mr-Neutr0n:fix/raise-not-implemented-error

Conversation

@Mr-Neutr0n
Copy link

Summary

  • NotImplemented is a built-in constant used for rich comparison methods, not an exception class. Raising it directly produces a confusing TypeError: exceptions must derive from BaseException instead of the expected NotImplementedError.
  • This fixes the else branch in wrap_llm_lora() to properly raise NotImplementedError with a descriptive message indicating which LLM architecture is unsupported.

Before (broken):

raise NotImplemented
# TypeError: exceptions must derive from BaseException

After (fixed):

raise NotImplementedError(f'Unsupported architecture: {self.llm_arch_name}')
# NotImplementedError: Unsupported architecture: SomeModel

Test plan

  • Verified that raise NotImplemented produces TypeError in Python
  • Confirmed this is the only instance of this pattern in the codebase
  • The fix follows the same pattern used elsewhere in the file (e.g., line 84)

`NotImplemented` is a built-in constant intended for use in rich
comparison methods, not an exception. Raising it directly produces a
confusing `TypeError: exceptions must derive from BaseException` instead
of the expected `NotImplementedError`.

This change fixes the error in `wrap_llm_lora()` to properly raise
`NotImplementedError` with a descriptive message showing which
architecture was unsupported.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant