refactor(audio-gen): apply backend review feedback to ThinkSound and ACE-Step#2594
Conversation
… error Applies the review feedback from lemonade-sdk#2580 to the merged ThinkSound and ACE-Step backends: separate ROCm and CUDA loader-path branches, an UnsupportedOperationException instead of a blind vulkan fallback when no compatible GPU backend is detected, and unnamed unused parameters instead of (void) casts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbpKMPt3rkoxTRJMaDwWXv
fl0rianr
left a comment
There was a problem hiding this comment.
Thanks for going after this.
Small wording / intent clarification: this change looks consistent with the existing backend-selection flow to me. It removes the unconditional fallback to vulkan when get_supported_backends() returns no candidates, while still allowing vulkan when system-info considers it supported.
One small nuance: Lemonade’s Vulkan support rows intentionally include cpu/x86_64 in some recipes as a proxy for generic Vulkan-capable systems / iGPU paths, not necessarily as a pure CPU backend. sd-cpp uses the same pattern, while also having a dedicated cpu backend.
So I think the code is fine, but the PR description / error text might be slightly clearer as “no supported backend detected” rather than “no supported GPU backend detected”. Otherwise it can sound like Vulkan should only count when an explicit AMD/NVIDIA GPU is detected, which is not quite how Lemonade models Vulkan today.
|
Aight, changed it :) |
fl0rianr
left a comment
There was a problem hiding this comment.
Was a bit nitty but I thought letting you know this had it's worth on its own. Approving.
Applies the review feedback from #2580 (superm1) to the already-merged audio-generation backends, matching the changes made on the OpenMOSS (#2580) and Trellis (#2581) PRs:
UnsupportedOperationExceptioninstead of silently defaulting to vulkan when no supported backend is detected(void)casts