fix: include user-defined LLM families in _resolve_architectures - #5351
fix: include user-defined LLM families in _resolve_architectures#5351Ricardo-M-L wants to merge 1 commit into
Conversation
The _resolve_architectures method only looked up architectures from BUILTIN_LLM_FAMILIES, which caused custom-registered models with a model_family pointing to a user-defined model to fail architecture resolution. Now combines both builtin and user-defined families. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the _resolve_architectures method in xinference/model/llm/llm_family.py to support user-defined LLM families alongside builtin ones. The reviewer pointed out a performance overhead issue where dictionaries are rebuilt on every call, and suggested optimizing this by first checking builtin families and lazily importing and checking user-defined families.
| from .custom import get_user_defined_llm_families | ||
|
|
||
| user_defined = {f.model_name: f for f in get_user_defined_llm_families()} | ||
| all_families = {f.model_name: f for f in BUILTIN_LLM_FAMILIES} | ||
| all_families.update(user_defined) | ||
| if self.model_family in all_families: | ||
| return all_families[self.model_family].architectures | ||
| return None |
There was a problem hiding this comment.
Rebuilding dictionaries for all builtin and user-defined families on every call to _resolve_architectures introduces unnecessary performance overhead, especially since builtin families are static and represent the vast majority of lookups.
Instead, we can first check BUILTIN_LLM_FAMILIES using a simple loop. If not found, we can then lazily import and check the user-defined families. This avoids the overhead of dictionary creation and imports for builtin models.
| from .custom import get_user_defined_llm_families | |
| user_defined = {f.model_name: f for f in get_user_defined_llm_families()} | |
| all_families = {f.model_name: f for f in BUILTIN_LLM_FAMILIES} | |
| all_families.update(user_defined) | |
| if self.model_family in all_families: | |
| return all_families[self.model_family].architectures | |
| return None | |
| for family in BUILTIN_LLM_FAMILIES: | |
| if family.model_name == self.model_family: | |
| return family.architectures | |
| from .custom import get_user_defined_llm_families | |
| for family in get_user_defined_llm_families(): | |
| if family.model_name == self.model_family: | |
| return family.architectures | |
| return None |
2 participants
This PR addresses: include user-defined LLM families in _resolve_architectures