Skip to content

Conversation

@StAlKeR7779
Copy link
Contributor

@StAlKeR7779 StAlKeR7779 commented Apr 19, 2023

Note: it based on #3184

I deleted a lot of classes/functions without any references.
But not sure - maybe some of this intended to be use in future, so if some of deleted parts needed - say, i'l restore.

Also renamed schedulers, as it's some confussion in names now with karras, and with dpmpp_2, as there exists multistep and singlestep versions(singlestep version in diffusers not ancestral)

Tested all function in web, so maybe missed some errors in cli.

StAlKeR7779 and others added 10 commits April 12, 2023 22:54
Imports:
`diffusers.models.cross_attention -> diffusers.models.attention_processor`

Unions:
`AttnProcessor -> AttentionProcessor`

Classes:
| Old name | New name|
| --- | --- |
| CrossAttention | Attention |
| CrossAttnProcessor | AttnProcessor |
| XFormersCrossAttnProcessor | XFormersAttnProcessor |
| CrossAttnAddedKVProcessor | AttnAddedKVProcessor |
| LoRACrossAttnProcessor | LoRAAttnProcessor |
| LoRAXFormersCrossAttnProcessor | LoRAXFormersAttnProcessor |

Same names in this class:
`SlicedAttnProcessor, SlicedAttnAddedKVProcessor`
@hipsterusername
Copy link
Member

@StAlKeR7779 - Definitely appreciate the updates! I think we're going to need to make these changes to main. Unclear that there will be another 2.3 version released.

@damian0815
Copy link
Contributor

damian0815 commented Apr 19, 2023

thanks for the work!

what happens when somebody wants to import parameters from an existing generation (using the old sampler names)? i couldn't tell from the diff whether the sampler names have changed at the python level or only in the UI

@hipsterusername
Copy link
Member

thanks for the work!

what happens when somebody wants to import parameters from an existing generation (using the old sampler names)? i couldn't tell from the diff whether the sampler names have changed at the python level or only in the UI

My read on this is that this change has made sampler selection explicit, in the sense that there are now k_ and non-karras versions of samplers, where supported

My estimation here is that old samplers which do not have a matching sampler in the current install would default to whatever sampler is currently selected (i.e., retrieving and setting the unsupported sampler would fail), but have not tested.

@blessedcoolant
Copy link
Collaborator

I think this change is better off being based against main as @hipsterusername pointed out.

Also why was the husky pre-commit config file removed? And this will most likely break the metadata retrieval for previous generations. We'll need to add manual checks to make those work.

@psychedelicious
Copy link
Contributor

ya pls restore the husky file

we need a map object of old sampler names to new.

fortunately, moving forward into nodes, the metadata format will change a bit, so we will be able to determine if the metadata is using the old names or new. having the map will let us correctly recall the parameters.

thanks!

@StAlKeR7779 StAlKeR7779 requested a review from GreggHelt2 as a code owner April 20, 2023 20:17
@damian0815
Copy link
Contributor

i'd like to take a look at how karras scheduling has been implemented, but there's 51 files here.. can you point me to where this change has been made in the inference code path?

@StAlKeR7779
Copy link
Contributor Author

StAlKeR7779 commented Apr 21, 2023

i'd like to take a look at how karras scheduling has been implemented, but there's 51 files here.. can you point me to where this change has been made in the inference code path?

ldm/generate.py:1142
https://github.com/invoke-ai/InvokeAI/pull/3232/files#diff-cc3b45afcc0ced92061f27eedf607c2382cd6cfba9b81a63c7190b31c070268eR1142

@damian0815
Copy link
Contributor

oh, it’s just a config flag on init! i thought it was gonna be way more complicated than that. great, thank you!

@hipsterusername
Copy link
Member

@damian0815 - Did you have a chance to review? Wondering if this is pending or needs any further changes

@StAlKeR7779
Copy link
Contributor Author

@hipsterusername I think it can be dropped now as this changes for 2.3

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.

6 participants