basic external code api

[ljleb]

trafficstars

I tried playing a bit with the idea of creating a module for external code/other extensions to use the extension. I tried rephrasing the web api implementation in terms of the external code module as a proof of concept.

WIP for now, I haven't tested that it doesn't break the web api.

Would love to have your opinion on the interface or implementation @scruffynerf as IIUC you wrote the code for something like this recently. Does the external_code.py module match your requirements?

closes #444

[ljleb]

One thing that I notice is that default values are duplicated a bunch of places in the code. I'm thinking of putting them in one place and override when needed to reduce risk of mistakes.

Also where should we add docs for this, in the wiki?

[scruffynerf]

I'll play with this tonight...

Quick take solely on looking over code: setting is most critical (and covered), but reading can also matter (ie what is the state of CN layers at a given moment), so some way to poll it. Again, it's super trivial now to look in the p.* and see the prompt, negative prompt, etc all set in the UI... But some way to do so for CN value is lacking here. My example code did both a Set the Layer, and a way to get/set particular values if desired. (I considered adding a Get the Layer, but realize mostly I cared about particular values, but likely long term, dumping the entire layer into a list or dict( with the names as keys) is probably the right answer.

More when I try it out...

[ljleb]

Thanks for your input! Sure I can add a "fetch args from script" function like you implemented earlier.

My intention was for users to create and manage their own list of ControlNetUnits, and only then, when they are ready to commit the changes, do they call external_code.update_cn_script_args. It's less risky to get conversion bugs like this IIUC. They can call it as many times as they want also, so committing the changes should not be a problem for code that calls it multiple times.

Though, if one extension wants to poll the arguments of an existing cn script, the "fetch args from script" could come in handy.

[scruffynerf]

One thing that I notice is that default values are duplicated a bunch of places in the code. I'm thinking of putting them in one place and override when needed to reduce risk of mistakes.

Also where should we add docs for this, in the wiki?

One thing about this that I realize my implementation also suffers from is fragility in the case of new CN UI changes, any new element/value or default change will require updating every place using that sort of "roll your own" So yes, centralizing it and making it easier to update in one place rather than multiple will hopefully enable future new or altered UI elements to just have simple updates to this as part of a PR. (This is of course the huge strength of doing it this way as a single CN library that other scripts can include [assuming CN is installed] ... Most of that code fragility is reduced to just ensuring CN UI changes are reflected with sane defaults within this new code)

[scruffynerf]

Might also make sense to expose ways to get model list, resize options, etc. Anyone wanting to change those sort of values is going to need to know what's valid.

I know model list is an available function now, but including a wrapper in this would make it easier to just import the one library

[ljleb]

I agree. Having to pass strings like "Scale to Fit (Inner Fit)" as args is very error prone and I heard the issue of i18n being raised. We should use something like an enum I think instead.

[ljleb]

I just found why model list is only updated in api between restarts. The cn_models_names dict is imported in api.py but the value is = {} instead of .clear() in controlnet.update_cn_models. Also the value is imported in different places and for some reason there are multiple dicts in memory, only one of them gets updated.

closes #413 (already closed but wasn't fixed)

[ljleb]

@Mikubill don't really wanna bother you just for this, but just to be sure, is it fine if I create a wiki page for this external_code module?

[Mikubill]

@Mikubill don't really wanna bother you just for this, but just to be sure, is it fine if I create a wiki page for this external_code module?

no problem just do it ;-)

[scruffynerf]

This is coming along really nicely. Will test once you feel it's gotten solid enough.

I spent last night writing a quick PR for the depthmap extension (nonCN) stereo image generation, and blown away by my results (future folding that functionality into CN makes sense, I suspect, likely thru this external code) so I was usefully distracted, and you've improved so much on this.

[ljleb]

Thanks for the help! If you want, I think you can test now, whenever you find some time. I ran the code a bit and rephrased some of the code to call this external_code module, so I am sure the main code paths are working. Feel free to suggest changes if you find something that isn't working 👍

[scruffynerf]

Trying to use this to replace my homebrew multicn code...

having lots of problems.

item 1: to import external_code in my script (which lives in an extension, so it's in extensions/myextension/scripts/myscript.py) only code I could get working (consistently, I had weirdness with other attempted methods)

import importlib
external_code = importlib.import_module('extensions.sd-webui-controlnet.scripts.external_code', 'external_code')

due to the dashes in sd-webui-controlnet, lots of the standard python methods will not work.

item 2: cn_models = external_code.get_models() returns empty results.

item 3

File "....../stable-diffusion-webui/extensions/sd-webui-controlnet/scripts/external_code.py", line 144, in update_cn_script_args
    script_args[script.args_from:script.args_to] = flattened_cn_args
TypeError: 'tuple' object does not support item assignment

I had the same issue in my homebrew, and ended up having to recast p.script_args as list, because of this. so in mine, I ended up with p.script_args = list(p.script_args[:script.args_from+1]) + list(new_args) + list(p.script_args[script.args_to:])

in trying this PR code out, I did: external_code.update_cn_script_args(p.scripts, p.script_args, updatedvalues) and got the above error... because by default, p.script_args is acting like a tuple, not a list, unless you change it.

so I changed your code to do this:

            script_args = list(script_args)
            script_args[script.args_from:script.args_to] = flattened_cn_args
            script_args = tuple(script_args)

just changed it to a list, then back to a tuple

And then... still nothing... and I realized that nothing seemed to be put the changed args back into p... so I added return script_args to the end of that function and then called it with p.script_args = external_code.update_cn_script_args(p.scripts, p.script_args, currentcn)

A better set of example code in the wiki is needed too, I really had to figure out what to do with the basics:

currentcn = external_code.get_all_units(p.scripts, p.script_args)
currentcn[0].enabled = True
currentcn[0].model = model
currentcn[0].image = img
p.script_args = external_code.update_cn_script_args(p.scripts, p.script_args, currentcn)

The model name wasn't working like before... and I still don't see a way to get the Resize enumerate unless I do it myself in my own code (ie it's not passed forward)

Disappointed after some hours fussing with this, and still not quite working. At least with my old code it worked. So it's not quite ready yet.

[ljleb]

Thanks for the feedback.

item 1

~~Sure I didn't run the importlib code, I'll update the wiki.~~ Done.

item 2

Can you try with external_code.get_models(update=True)? If the list is not up to date in memory already, you have to fetch it from disk. Default of update is False because by default I thought it would be better to not fetch disk all the time. I can change the default value maybe?

item 3 And then... still nothing... and I realized that nothing seemed to be put the changed args back into p...

As you make a copy of the input list, a temporary list is updated instead of the original list. We just need to also accept tuples in the function, although I don't think it will be possible to resize script_args in this case. Is the args a tuple by default? In the api route of the webui, they create p.script_args as a list so I assumed it would always be a list.

We could go the immutable route and return a new list instead of mutating it in place I guess. That would allow for tuples to work with the code as well. One problem with this is that we need to offset the args_from and args_to properties of all scripts if we update script_args, as we are splicing the arguments of as many processing units as we need into script_args which changes its size.

[ljleb]

I think I'll add a new function that works directly with p instead.

[scruffynerf]

sigh, in reverting things, I was like... wait, where's my models.... there are no models... and then realized that I'd moved my existing controlnet extension out, and git cloned your repo in for the PR... and of course my models were in a folder IN the extension, so I was testing with no models. That would explain both the lack of models in the list, AND the lack of results, once I had things fixed otherwise. I guess I need to retry some stuff.

[scruffynerf]

Is the args a tuple by default? In the api route of the webui, they create p.script_args as a list so I assumed it would always be a list.

I believe that's true, would be nice to confirm from others... but it seems like it is for me.

One problem with this is that we need to offset the args_from and args_to properties of all scripts if we update script_args, > as we are splicing the arguments of as many processing units as we need into script_args which changes its size.

Oh, I assumed that if 5 controlnet layers/unit were enabled, those were reserved space, so moving things around wouldn't matter. Since you can't change the # without a settings change (and restart?), I thought there wasn't a need to move stuff around.

In fact, I think I'm right here, as I was noticing the later addons breaking with your code... as in 15 args shifted... when they shouldn't have? Can you check that?

[scruffynerf]

yeah, moved my old models from the model directory in the old extension (thankfully I'd not deleted it), and into the /models/ControlNet directory, and the models are back. Whew.

[ljleb]

In fact, I think I'm right here, as I was noticing the later addons breaking with your code... as in 15 args shifted... when they shouldn't have? Can you check that?

My intention was to allow for an arbitrary number of scripts to be passed to controlnet. The maximum number of controlnet units limitation is only a ui limitation, as gradio does not allow to create new ui elements on the fly. We could resize the arguments to their minimum size I suppose at the very least. I'm just trying to keep the code as permissive as possible, maybe someone will find a use to generating controlnet parameters on the fly with arbitrary number of controlnet processing units, I don't know.

By the way as python does not allow overloading, I've had a hard time with function names 😅 I'll try to find something better than update_cn_script_args_impl. Not sure if "processing unit" is the right terminology also, I have seen "layer" being thrown around a lot. Maybe it would be a better idea to use layer.

[ljleb]

That should cover all the feedback. If you still have a bit of energy left for this, let me know if it works better now. I added a function update_cn_script_in_processing to update a StableDiffusionProcessing, as opposed to update_cn_script_in_place that was available before. (still available, but now you can simplify your code a bit I think with this)

[scruffynerf]

broken in a new way:

File "....SD/stable-diffusion-webui/extensions/sd-webui-controlnet/scripts/external_code.py", line 106, in update_cn_script_args
    script_args = list(p.script_args) if p.script_args else []
AttributeError: 'ScriptRunner' object has no attribute 'script_args'

[ljleb]

Yeah I realized it wasn't right. Please try latest commit. I think you can use this function now:

def update_cn_script_in_processing(
    p: processing.StableDiffusionProcessing,
    cn_units: List[ControlNetUnit],
    is_img2img: Optional[bool] = None,
    is_ui: Optional[bool] = None
):

To call it:

external_code.update_cn_script_in_processing(p, units)

[scruffynerf]

better, but still getting the error about other addons getting hosed:

Error running process: SD/stable-diffusion-webui/extensions/stable-diffusion-webui-daam/scripts/daam_script.py
Traceback (most recent call last):
  File "SD/stable-diffusion-webui/modules/scripts.py", line 386, in process
    script.process(p, *script_args)
TypeError: Script.process() missing 10 required positional arguments: 'attention_texts', 'hide_images', 'dont_save_images', 'hide_caption', 'use_grid', 'grid_layouyt', 'alpha', 'heatmap_image_scale', 'trace_each_layers', and 'layers_as_row'

Error running process: SD/stable-diffusion-webui/extensions/stable-diffusion-webui-two-shot/scripts/two_shot.py
Traceback (most recent call last):
  File "SD/stable-diffusion-webui/modules/scripts.py", line 386, in process
    script.process(p, *script_args)
TypeError: Script.process() missing 5 required positional arguments: 'enabled', 'raw_divisions', 'raw_positions', 'raw_weights', and 'raw_end_at_step'

Error running process_batch: SD/stable-diffusion-webui/extensions/stable-diffusion-webui-daam/scripts/daam_script.py
Traceback (most recent call last):
  File "SD/stable-diffusion-webui/modules/scripts.py", line 395, in process_batch
    script.process_batch(p, *script_args, **kwargs)
TypeError: Script.process_batch() missing 10 required positional arguments: 'attention_texts', 'hide_images', 'dont_save_images', 'hide_caption', 'use_grid', 'grid_layouyt', 'alpha', 'heatmap_image_scale', 'trace_each_layers', and 'layers_as_row'

the fact that's it's 15 aka one unit length, tells me your code isn't right.

also still needs class added?

ResizeMode(Enum):
    RESIZE = "Just Resize"
    INNER_FIT = "Scale to Fit (Inner Fit)"
    OUTER_FIT = "Envelope (Outer Fit)"

so we can set that.

[ljleb]

~~Hmm maybe an off-by one. I'll test the whole thing before asking for you to check that it meets your requirements now.~~

Not an off by one, I can't reproduce. I wish I knew what causes it to offset the remaining script args. (maybe script_args originally empty and not a coincidence?)

also still needs class added?

I think you can just use external_code.ResizeMode no?

[ljleb]

Btw is p.script_args empty before the call to update_cn_script_in_processing on your side, or does it have already filled arguments? The code assumes all script args are already pre-filled with some value.

[scruffynerf]

I think you can just use external_code.ResizeMode no?

ah, that wasn't clear to me, ok, definitely that needs documenting in code, if not in wiki At least with the models, it's clear there is a function for getting the names.

So... currentcn[0].resize_mode = external_code.ResizeMode.RESIZE ? (or INNER_FIT or OUTER_FIT)

[scruffynerf]

Btw is p.script_args empty before the call to update_cn_script_in_processing on your side, or does it have already filled arguments? The code assumes all script args are already pre-filled with some value.

it's got everything.

[scruffynerf]

it works, except for the above 15 argument issue still.

[ljleb]

Do you know if it's the scripts before or after that break? I assume it's the scripts that are listed after controlnet in p.scripts.awlayson_scripts.

[scruffynerf]

I'll add some debug, one sec...

[ljleb]

Thanks a lot for your time by the way. It makes the api more suitable for everyone and I appreciate it!