fixes sampler.name; refactored apply_sampler, confirm_sampler, build_sampler_dict; valslist ready to apply when returned from process_axis #4909
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tested and seems to work @AUTOMATIC1111 this would be an important fix as its currently broken |
apply_sampler should not build or check for errors, it should
only apply the sampler to the process handed to it.
process_axis, when axis was "Samplers", did not finish processing
the valslist. For this, I implemented build_samplers_valslist.
build_samplers_dict was being called twice, once in apply, and once
in confirm_samplers, I pulled this out from confirm and put it into
an if opt.label == samplers block in process_axis
build_samplers_dict was modified to return a dictionary with a different
structure.
I believe this new structure makes it more clear for readers.
The previous structure was a dictionary where the first key was the
lowercase version of sampler.name, and adjacent was a key that was an
alias, both set with the same int value, for all samplers.
Because dictionaries are now ordered, dict.get(val) would return the first
match.
Also, this structure did not include the proper name needed by
Process.sampler_name. The structure is now
{Sampler.name: [Sampler.name.lower(), alias1, ...], }
Now you can check if user value is in set of flattened dict.values(),
"set(chain(*dict.values()))"
and grab Sampler.name from the key directly.
The building of valslist, utilizes this structure, function is
build_samplers_valslist
It iterates through (previously confirmed) user supplied values,
steps through each dict.values until it matches, then appends
the proper name from key into list
|
Code now works for aliases from sampler.aliases; and sampler.name; case insensitive. Did a refactor of apply_sampler, confirm_sampler, and build_samplers_dict. |
Gerschel
changed the title
Gerschel
changed the title
AUTOMATIC1111
added a commit
that referenced
this pull request
Nov 27, 2022
|
well done with spotting and fixing the problem but i don't welcome refactors and i disagree with using a dictionary this way; I uploaded my fix so I will be closing this |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sorry, I had a long week.
Final try to do a pull request. If I screw it up, going to bed.
#4860
Fixes apply_sampler() using name instead of index while respecting the function and keeping case insensitivity.