refactor: attention backend registry#256
Conversation
![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 refactors the attention backend and pipeline registries by consolidating them into a unified registry module (diffsynth_engine/registry.py) and transitioning AttentionType to a string-based enum. It also replaces the hardcoded ring attention compatibility list with a dynamic supports_ring_attention capability check on the backend classes. The reviewer's feedback suggests improving error messages when attn_type is None by dynamically retrieving the resolved backend name, and making the backend selection case-insensitive by converting attn_type to lowercase in the registry.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| attn_backend = get_attn_backend(attn_type) | ||
| if not attn_backend.supports_head_size(head_size): | ||
| raise ValueError(f"Attention backend {attn_type!r} does not support head size {head_size}.") |
There was a problem hiding this comment.
If attn_type is None (the default), the error message will display None instead of the actual resolved backend name (e.g., 'sdpa'). Using attn_backend.get_type() ensures the correct backend name is shown in the error message.
| attn_backend = get_attn_backend(attn_type) | |
| if not attn_backend.supports_head_size(head_size): | |
| raise ValueError(f"Attention backend {attn_type!r} does not support head size {head_size}.") | |
| attn_backend = get_attn_backend(attn_type) | |
| if not attn_backend.supports_head_size(head_size): | |
| raise ValueError(f"Attention backend {attn_backend.get_type()!r} does not support head size {head_size}.") |
| attn_backend = get_attn_backend(attn_type) | ||
| if not attn_backend.supports_head_size(head_size): | ||
| raise ValueError(f"Attention backend {attn_type!r} does not support head size {head_size}.") |
There was a problem hiding this comment.
If attn_type is None (the default), the error message will display None instead of the actual resolved backend name (e.g., 'sdpa'). Using attn_backend.get_type() ensures the correct backend name is shown in the error message.
| attn_backend = get_attn_backend(attn_type) | |
| if not attn_backend.supports_head_size(head_size): | |
| raise ValueError(f"Attention backend {attn_type!r} does not support head size {head_size}.") | |
| attn_backend = get_attn_backend(attn_type) | |
| if not attn_backend.supports_head_size(head_size): | |
| raise ValueError(f"Attention backend {attn_backend.get_type()!r} does not support head size {head_size}.") |
| if attn_type is None: | ||
| attn_type = "sdpa" | ||
| if attn_type not in ATTENTION_BACKEND_REGISTRY: |
There was a problem hiding this comment.
To make the attention backend selection case-insensitive and more robust against mixed-case string inputs (e.g., 'SDPA' or 'FA2'), we should convert attn_type to lowercase when it is not None.
| if attn_type is None: | |
| attn_type = "sdpa" | |
| if attn_type not in ATTENTION_BACKEND_REGISTRY: | |
| if attn_type is None: | |
| attn_type = "sdpa" | |
| else: | |
| attn_type = attn_type.lower() | |
| if attn_type not in ATTENTION_BACKEND_REGISTRY: |
1 participant
No description provided.