Skip to content
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

LoRA bundled TI infotext #15679

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

LoRA bundled TI infotext #15679

wants to merge 1 commit into from

Conversation

w-e-w
Copy link
Collaborator

@w-e-w w-e-w commented May 1, 2024

Description

alternative implementation of

differences

  • as opposed just to writing bundled to infotext TI hashes, write the LoRA name as the hash to indicate its origins

imo writing bundle alone is not very useful if you have multiple loras

  • add an option for user to toggle this behavior (default enable)
    setting key : lora_bundled_ti_to_infotext
    description:
Add Lora name as TI hashes for bundled Textual Inversion ("Add Textual Inversion hashes to infotext" needs to be enabled)

as onec a LoRA is loaded is is cashed and won't be reloaded
using a custom BundledTIHash class allows for the string dynamically change based on settings with no change to other code


things to consider

  • maybe instead of putting the lora name, put the hash of the lora
  • alternatively maybe calculate the real hash of the bundled TI
  • if the lora name is used should we add some sort of prefix to distinguish edge case when if someone use a hash like string for there lora name

@mx any opinion on this implementation

damn that's a short username how did you get such a short username

Checklist:

Co-Authored-By: Morgon Kanter <9632805+mx@users.noreply.github.com>
@w-e-w w-e-w requested a review from AUTOMATIC1111 as a code owner May 1, 2024 10:59
@mx
Copy link

mx commented May 1, 2024

@mx any opinion on this implementation

Seems fine to me, I'll withdraw my PR. Do we even need the additional setting though? The user already has to opt-in to a setting in order to have the TI hashes appear in the infotext at all.

@w-e-w
Copy link
Collaborator Author

w-e-w commented May 1, 2024

Seems fine to me, I'll withdraw my PR. Do we even need the additional setting though? The user already has to opt-in to a setting in order to have the TI hashes appear in the infotext at all.

this is my line thought

in a way bundled TI 's are are not as important
as in a way it is a part of the model, they will never be separated by the average user
so for the average user doesn't really need to care if there's a hidden TI they or not
and they might not want to be spammed with the extra extended TI infotext

in fact I'm actually considering making disable as default

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.

None yet

2 participants