TensorStack-AI / OnnxStack

C# Stable Diffusion using ONNX Runtime
Apache License 2.0
221 stars 33 forks source link

Added some common exceptions and guards #94

Closed jeffward01 closed 9 months ago

jeffward01 commented 10 months ago

Added to StableDiffusionService class mainly.

The reason why I added these guards is because in the examples, i fixed a bug where the wrong scheduler was used. This lead me to explore schedulers and see that some multiple checks were performed. I wanted to add some strongly typed classes to add to readability and understanding.

Feel free to reject this pull request if you do not think it is helpful or dislike the additional classes.

Thanks!

jeffward01 commented 10 months ago

A few of my commits are in this branch that are from another PR: https://github.com/saddam213/OnnxStack/pull/93

If there are issues with this merge let me know and I will sort out the version control. Sorry for the branches

saddam213 commented 10 months ago

Thanks for the contributions.

Very happy with the new Exceptions, one of the many things on my TODO list :)

Regarding the Guard classes, the code base is changing a bit fast to start adding too many of these, so if possible if we could leave those out for now and we can revisit after the main API has been finalized a bit, save us fixing them up every 10 mins.

Also when we get to it, handling Guards inside exceptions if possible would be preferred, seems to be the way Microsoft is now doing it e.g: ArgumentException.ThrowIfNullOrEmpty("Name")

DiffuserNotFoundOrSupportedException.ThrowIf(pipeline, diffuserType);

By version v0.20.0 the APIs should be in a much better state to add some sanitation and guarding things.

Happy to merge the rest if you would like to make those few edits :) and thanks again

saddam213 commented 10 months ago

Regrading PipelineSchedulerGuard, you could use Enum.GetValues to simplify the dictionary here, no reflection needed

var _pipelineSchedulerConfigs =  Enum.GetValues<DiffuserPipelineType>().ToConcurrentDictionary(x => x, k => k.GetSchedulerTypes());
saddam213 commented 9 months ago

After the API rewrite the StableDiffusionService no longer exists, PR is now obsolete :(