-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix(ui): Fix related models in new model picker. #8151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like there are two broad changes in this PR:
- Display only LoRAs that are compatible with the currently-selected main model.
- Add an asterisk character in front of the LoRAs that are flagged as related to the currently-selected main model.
I don't think this is the right approach.
- We recently added the ability to provide initial enabled groups to the model picker. Perhaps we should use that instead of providing only the compatible models to the picker. Elsewhere in the app, we do not restrict available options by model base. We show them all, disabling the incompatible ones. I could be convinced it's better to hide them entirely, but initial enabled groups solves it.
- Instead of adding all this extra logic to wrap the Picker component and logic with related model stuff, how about we just add a
starred?: boolean
field to the Picker options:- When we create the list of model options, add
starred: true
to the LoRAs that are flagged as related. - Sort the model options so that the related ones are first, before passing them into the ModelPicker.
- Update the Option component in the Picker to accentuate the
starred
options. Maybe a little yellow star next to the name.
- When we create the list of model options, add
I made the explicit ask to move away from the recent "show all groups", namely because there's no real value in seeing LoRAs that you can't enable with the current base model. I can't see that it's clearer to have filtering functionality, and while the styling looks nice, I think it's probably clearest for novice users to think of the "model" as the defining variable in all other settings they see, rather than the complex web of compatibility we're managing. Thoughts? (Is fixed otherwise) |
I don't really mind whether we disable incompatible LoRAs (and disabling their architecture groups by default), or omit them entirely. But I do think there is value in having them all visible - you can see all your options. If you forget which model has supports your fav LoRA, you can enable all groups and figure that out. |
invokeai/frontend/web/src/features/parameters/components/ModelPicker.tsx
Outdated
Show resolved
Hide resolved
invokeai/frontend/web/src/features/parameters/components/ModelPicker.tsx
Show resolved
Hide resolved
invokeai/frontend/web/src/features/parameters/components/ModelPicker.tsx
Show resolved
Hide resolved
invokeai/frontend/web/src/features/parameters/components/ModelPicker.tsx
Outdated
Show resolved
Hide resolved
invokeai/frontend/web/src/features/parameters/components/ModelPicker.tsx
Outdated
Show resolved
Hide resolved
cf232b3
to
534df83
Compare
534df83
to
9d78c48
Compare
Summary
LoRA is using the new model picker component with icons, but hides unnecessary filters that overcomplicate it. Related models are now called out.
Related Issues / Discussions
Discussion re: related model component functionliaty
QA Instructions
Open the LoRA model and ensure it is effectively working w/ the model base you have selected.
Merge Plan
Merge is go.
Checklist
What's New
copy (if doing a release after this PR)