mcmonkeyprojects / sd-infinity-grid-generator-script

Infinite-Axis Grid Generator for Stable Diffusion!
MIT License
180 stars 24 forks source link

Feature request: skip checkpoint if it is not found in list #99

Closed hakaserver closed 1 year ago

hakaserver commented 1 year ago

In my specific case, I have a long grid for comparing dozens of models against a few example prompts. When passing a list of checkpoints in the simplified format (model: "list, of, models"), if the model is not found in the list of available checkpoints (old model removed for example) it errors out with:

"RuntimeError: Invalid axis 'model': errored: value'modelnamehere.safetensors' errored: Invalid parameter 'model' as 'None': not matched to any entry in list ['list','of','models']

If you simply remove the model from the list, obviously the grid would be mislabeled. From what I understand, you could use 'skip: true' so that any removed models would be skipped, but that requires that the yaml file be explicitly configured for each model to be used in the axis (again, from what I understand), such as in:

axes:
 model:
  title: Models
  valus:
   model_a:
    title: model_a
    params:
     model: model_a.safetensors
    skip: true
   model_b:
    title: model_b
    params:
     model: model_b.safetensors
    skip: false

However, I think it would be easier if we had the option to simply skip a value if it is not found in the model list. That way you could still pass the list in the simplified format and just add to it, while also being able to not keep all old models in disk.

Is there any other way of doing this that I do not know? Else I think this feature would be useful.

yggdrasil75 commented 1 year ago

why this occurs: validate_params is called at line 280 for each key. this should be changed to have "if not self.skip: validateparams" it is also called at 375 for everything so it adds overhead in addition to breaking if you remove a model from your drive but still want to keep the model as an option in your page.

I would also note, validateparams does not actually apply the params, it just checks if they are valid. since that is the case, you can put it in a separate thread, speeding up the process a bit. on my branch for comparison: https://github.com/yggdrasil75/sd-infinity-grid-generator-script/blob/Dev/gridgencore.py#L348 the dev branch here has a lot of things that improve speed and add functionality. but mostly its still untested on if it truly is faster, or just faster in theory.

mcmonkey4eva commented 1 year ago

I tested before and blindly throwing python threads at things doesn't end up notably faster, even the easy/obvious cases like image saving in its own thread makes very minimal overall speed impact - in some cases it can make perf worse or even add bugs if you're not very careful about how you do it.

The Swarm version of inf grid does actually achieve noticeably faster perf (extra so if you have multiple backends, but even with just 1 comfy backend it does a pretty good job of optimizing throughput rate) through more carefully controlled multithreading (and through a more generally performance-oriented language, C#)

Line 375 doesn't call it "for everything", it validates the root params in the yaml if those are set.

Your fork makes a long list of changes that seem to be based on misunderstandings of the original code, I would advise significantly more caution in making changes like that.

yggdrasil75 commented 1 year ago

the swarm version is csharp. converting this to python with clr would see speed improvements even with the exact same functionality. python is one of the worst languages for speed, but it is used widely for frontend ai anyway. there is a reason why kobold and llama switched away from python to kobold.cpp and llama.cpp. stablediffusion.cpp is faster for cpu, but I havent seen it pushed to cublas to compare on gpu yet.

also, that comment about saving in its own thread is the reason why saving is currently not in its own thread in mine. I know how to fix, but havent yet. probably will tomorrow.

the version I am working on is faster than this one by quite a bit if only for 1 reason: batching. a batch of 36 is .10 s/it/im but a batch of 1 is .45s/it/im on my setup. in my fork, a skipped model is ignored. it works fine without issue. please verify why yours pays so much more attention when it is not that hard to fix.

and for my fork making so many changes that are supposedly misunderstandings in the original code, just because I am not a python dev does not mean that I dont know how to code. my version is currently running. I am currently running the dev branch, I have gone through a large yaml with a total size of over 4 trillion generations, used normal skipping to skip around half of them, then used the piecemeal skipping to skip it down to around 900k, and it is running perfectly fine 5 hours later with no issues regarding bad generations, saving improperly, or anything the only further changes that arent currently pushed to the wrong output or any other issues. please state what you mean by misunderstandings of the original code.

hakaserver commented 1 year ago

I'm just a beginner, but that felt kinda petty. For now, I'll be switching to that 'off-topic' fork as it fixes my issue of checkpoints not skipping when not found.

mcmonkey4eva commented 1 year ago

Added a check to skip validation of params that have skip: true which is the first part of this feature request.

The comments above were marked off-topic (and 1 deleted) because they were off topic. (Does batching performance relate to checkpoint skipping? No, it's off topic) Additionally, yggdrasil75 claiming that line 375 is relevant was at least on-topic, but was incorrect, and the back-n-forth about that was irrelevant to the issue.

mcmonkey4eva commented 1 year ago

Added the other half, either skip_invalid: true in the yaml -or- check the new "skip invalid" checkbox in the UI to autoskip invalid entries

hakaserver commented 1 year ago

The new options doesn't seem to have any effect in the use case I mentioned. Either marking the new UI option, or adding any of the arguments mentioned on the yaml file works. It still shows the same error:

RuntimeError: Invalid axis 'model': errored: value 'model_erased_from_disk.safetensors' errored: Invalid parameter 'model' as 'model_erased_from_disk.safetensors': not matched to any entry in list.

Just so you actually understand the use case, here is the yaml file:

  title: my_checkpoint_grid
  description: 'Grid for comparing checkpoints'
  format: png
  author: Unspecified
  show descriptions: false
  autoscale: true
  sticky: false
  x axis: seed
  y axis: model
  x super axis: 'None'
  y super axis: 'None'
  params:
    Sampler: DPM++ 2M Karras
    VAE: vae-ft-mse-840000-ema-pruned
    Styles: Random_Wildcards
    Seed: 3309936095
    Steps: 20
    CFG Scale: 7
    Width: 512
    Height: 768
    Clip Skip: 2
axes:
  Model: "list, of, models, separated, with, commas, DELETED MODEL, new, models, added, to the, list"
  Seed: 3309936095, 1389736456, 1476840776, 2668476902, 2100126438

Selecting this yaml file and marking the new UI option doesn't work. Editing it as I did below also doesn't work:

axes:
  Model:
    title: Model
    skip_invalid: true
    skip: true
    values:  "list, of, models"
  Seed: list, of, seeds
mcmonkey4eva commented 1 year ago

skip_invalid: true goes in the grid definition, alongside title/description/format, as is shown in the example file - https://github.com/mcmonkeyprojects/sd-infinity-grid-generator-script/blob/master/assets/short_example.yml#L40-L41

hakaserver commented 1 year ago

Testing with skip_invalid: true now in the correct location still gives the same result:

grid:
  title: my_checkpoint_grid
  description: 'Grid for comparing checkpoints'
  format: png
  author: Unspecified
  show descriptions: false
  autoscale: true
  sticky: false
  x axis: seed
  y axis: model
  x super axis: 'None'
  y super axis: 'None'
  skip_invalid: true
  params:
    Sampler: DPM++ 2M Karras
    VAE: vae-ft-mse-840000-ema-pruned
    Styles: Random_Wildcards
    Seed: 3309936095
    Steps: 20
    CFG Scale: 7
    Width: 512
    Height: 768
    Clip Skip: 2
axes:
  Model: "list, of, models, separated, with, commas, DELETED MODEL, new, models, added, to the, list"
  Seed: 3309936095, 1389736456, 1476840776, 2668476902, 2100126438

RuntimeError: Invalid axis 'model': errored: value 'DELETED MODEL.safetensors' errored: Invalid parameter 'model' as 'DELETED MODEL.safetensors': not matched to any entry in list.

mcmonkey4eva commented 1 year ago

Oops, that's my bad - I implemented it for full-form yamls but not for comma-separated-list format. It should be all good now.