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

fixed likely error in MID12_50 preset #20

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

Conversation

Gamekiller48
Copy link
Contributor

I assume the name MID12_50 refers to the middle 12 blocks + M00 block being set to 50%? At the moment it's 13 blocks (7 In, 6 out) + M00, rather than 12 blocks (6 in, 6 out).

I assume the name MID12_50 refers to the middle 12 blocks + M00 block being set to 50%?
At the moment it's 13 blocks (7 In, 6 out) + M00, rather than 12 blocks (6 in, 6 out).
@bbc-mc
Copy link
Owner

bbc-mc commented Jan 21, 2023

Hi, thanks for the PR.

Yes, you are right.
MID12_50 means that IN06 to IN11 (6 blocks) will be 50%.

However, we need to consider if someone is already using this preset.
What about creating a "MID12_50fix" with correct values?

@Gamekiller48
Copy link
Contributor Author

Gamekiller48 commented Jan 21, 2023

https://xkcd.com/1172/

While the idea of respecting everyone's workflow is sound in theory, there are special circumstances to consider:

  • Block merging has a high barrier of entry, most if not all of its users can be considered technically affine power-users. Unlike with software targeted at the wider audience, one can expect power-users to react to and mitigate such changes, as long as they are properly communicated in a changelog.
  • Stable diffusion and the webui in general are a high velocity field, where things break daily anyways. People share models and recipes online that become obsolete the very next day. People are more understanding of changes that break existing workflows in this area than they might be in other fields.
  • This project is very early in adoption. With 74 stars so far (compared to 29k stars on the Automatic1111's repo or several hundred stars on some other scripts), changes like these can still realistically be made, without a single person actually noticing. This becomes considerably harder later on.

My personal recommendation would be to go for simplicity and to correct the error in the preset directly (as well as this image: https://github.com/bbc-mc/sdweb-merge-block-weighted-gui/blob/master/misc/preset_grid/MID12_50.PNG), rather than introducing new presets.
Technically my earlier commit where I fixed those missing 0s also could've broken the workflow of some people, as the script would just skip the incorrect values and leave the default values, often-times leaving M00 stuck at 0.5 rather than setting it to 0 or 1.
If we considered the possibility that people depended on this broken version, we would also need to create a SMOOTHSTEP*2_fix, SMOOTHSTEP*4_fix, SMOOTHSTEP/2_fix, but then what about people who downloaded yesterday's commit and already started their own workflow on these new versions...

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.

2 participants