search

Sygil-Dev / stable-diffusion

GNU Affero General Public License v3.0
1.72k stars 148 forks source link

range based sampling #64

Closed ulbg closed 2 years ago

ulbg commented 2 years ago

range based sampling cancel button info for all the generated images constant seed option

keep in mind i removed: demo.queue(concurrency_count=1) i don't think it is needed

hlky commented 2 years ago

Thank you for the effort, but for the following reasons I can't accept it right now.

Firstly it seems you haven't paid attention to the giant text about the development repo which should be used for development.

demo.queue(concurrency_count=1)

You shouldn't be removing or changing anything unrelated to your own changes. If you don't know why it's there or why it's done in a certain way, leave it alone.

Your PR is actually 4 features that aren't particularly related, if they were split they can be reviewed better, and reverted easier if required in the event of any bugs. It also makes it easier to know what is changed in each commit which helps track down where something has broken.

To clarify, submit separate pull requests on the development repo for each of your added features, with only the code for that feature in each.

Also, document the intended behavior of each feature, some are obvious but until I actually review the code I'm not sure exactly what you mean by ranged based sampling.

ulbg commented 2 years ago

I just realized I made the PR for the wrong repo as well. Alright I will split it up.

demo.queue(concurrency_count=1)

this is needed for the cancel button to work, but we can discuss it in the other PR