Skip to content

Implements Script order / Callback order#14220

Closed
aria1th wants to merge 5 commits intoAUTOMATIC1111:devfrom
aria1th:dev-script-order
Closed

Implements Script order / Callback order#14220
aria1th wants to merge 5 commits intoAUTOMATIC1111:devfrom
aria1th:dev-script-order

Conversation

@aria1th
Copy link
Collaborator

@aria1th aria1th commented Dec 6, 2023

Description

Related:#13943

on_*(callback, priority_getter_or_int)

Now, on_* type callback register supports one additional (optional) parameter, priority.

on_ui_settings(setup_ui, 10) -> Registers callback with priority 10 (bigger executes first)

on_ui_settings(setup_ui, lambda x:10) -> Registers callback with closure

Behavior:

at callback call time, we call the priority getter (which will handle callable or int) to get priorities, and sort by them.

As a result, we can control the order of callback execution.

Note that the callable currently does not pass any arguments.

Script callbacks

class ScriptHypertile(scripts.Script):
    name = "Hypertile"
    process_priority = -1000
    before_hr_priority = -1000
    
    def title(self):
        return self.name

    def show(self, is_img2img):
        return scripts.AlwaysVisible

    def process(self, p, *args):
        hypertile.set_hypertile_seed(p.all_seeds[0])

        configure_hypertile(p.width, p.height, enable_unet=shared.opts.hypertile_enable_unet)

        self.add_infotext(p)

    def before_hr(self, p, *args):

        enable = shared.opts.hypertile_enable_unet_secondpass or shared.opts.hypertile_enable_unet

        # exclusive hypertile seed for the second pass
        if enable:
            hypertile.set_hypertile_seed(p.all_seeds[0])

        configure_hypertile(p.hr_upscale_to_x, p.hr_upscale_to_y, enable_unet=enable)

        if enable and not shared.opts.hypertile_enable_unet:
            p.extra_generation_params["Hypertile U-Net second pass"] = True

            self.add_infotext(p, add_unet_params=True)

This is the modified script example, which would make hypertile process be always processed after priority 0 executions.
image

Screenshots/videos:

Checklist:

@w-e-w
Copy link
Collaborator

w-e-w commented Dec 6, 2023

some differences between my implementation #13943

  • users can override the default priority sets by the developer

  • the sorted results are stored as opposed to sorting them in realtime
    (which I believe would lead to a slight performance Improvement)

  • I used the term order as opposed to priority because there's already a order value used by the post processing script
    a post processing script with a lower order would be executed first

  • system that assigns a different order value based on the type of script if none is specified

  • I choose to add the order number as a property of the function because I feel like it's cleaner, maybe it's not I'm not sure


one thing that I don't have implemented is the callback order

I've only implemented script function order but did not implement


if requested I can rework my origin implementation

@aria1th
Copy link
Collaborator Author

aria1th commented Dec 6, 2023

@w-e-w I don't really expect users to modify the callback execution order - most of the case, they would not know anything about how extensions were written and working... rather, extensions creator should handle if the race condition happens.

Also it would be beneficial to have closures / functions that returns dynamic values at runtime, which can be effective for conditional ordering. We won't really benefit unless someone has 100k extensions that might take seconds to sort 🧠, so I'd suggest dynamic sorting for that, unless it includes some I/O operation.

For priority or order, order should be better choice with most simple word.

As far as I experienced, usually just a simple ordering / or conditional ordering is enough, maybe we already have DAG so expanding it as metadata....can be some option too.

So I would consider 2 options, as metadata (maybe readable and unified), or as code snippet style (arguments or closures, closures can be very dynamic but I think just 'Before' 'After' handling should be enough, and escaping scope to get all scripts name will be pain.....)

@w-e-w
Copy link
Collaborator

w-e-w commented Dec 6, 2023

user overridable is kind of important, even though the average user will not be able to utilize the system it is still beneficial to have

the TLDR it's that can be very beneficial for special circumstances and advanced users, read below if you want examples


also just want to make it clear that, I don't really have an objection of sorting the order at runtime, I'm just explaining the difference between our implementations and why I did it that way

I don't really have a preference between order or priority but I just think it might be better to keep it consistent


Examples

let's say there are two independent extensions that will function perfectly well without setting a specific order, because of which the extension developer did not set a custom order

later down the line a 3rd extension was made by some other developer, due to special reasons this extension needs to be executed in between the two extensions

because of those extension doesn't have a custom order they are both assign the same default order
this makes it not possible for this extension to inject itself between the two extensions above,

if this only happens for one function then it's still salvageable, because the developer can just set a custom extension load order using the DAG system, but if there are multiple functions or if this type of complex happens for multiple extensions there are certain combinations that are not possible to achieve simply by the load order

I realized this type of situation may not be common but it can hypothetically happen

and they're also cases that certain extension combination would have execution issues, let the developer simply does not know about (if the developer is made aware of the situation then this could be fixed potentially)

if issues like these occur obviously the best solution would be for the extensions to just update but sometimes this might require multiple extensions to be updated due to the same issue which is not exactly fast and easy to do

so it's more practical to allow the user to manually the order, and if the future fix is built into the extension they can always remove the override


they're also cases that the execution order is determined not by functionality but of user use case / preference

thats say prompt fusion and dynamic prompts both extensions provide related but different functionality

note I'm not a user of either of these extensions so these are just examples

one user might want to use prompt fusion to generate dynamic prompts syntax and the dynamic prompts to its thing, while
another user may wish to do the opposite

in this situation there is no one right way of configuring the execution orde, it depends on the user's use case


if you have two post processing operation and the output would be affected by the order that they are applied, and assume that both results are valid effects, in this case the developers cannot know beforehand what's the user wants and so cannot set a correct order because there is no correct order


in other cases such as the before / after component function

if two extensions both want to inject their own UI at the same position the user should have the choice to choose the order that the injected elements are placed
same goes for extension accordion order (but ideally this would be handled using a different system, as opposed to directly configuring the order)

@aria1th
Copy link
Collaborator Author

aria1th commented Dec 6, 2023

@w-e-w That makes sense, I was just trying to handle functionality setups - to just allow devs to plan and calculate their order, but those type of processing has to be considered too.

But still, I guess those customization has too many options to implement - for example, we can let extensions implement their API endpoints, and let someone combine them as processors...

My implementation was just for minimal stuff to specifying execution orders - just to avoid hijack hellness... so we might want separate discussion about this, maybe comfyUI-style node conversion / etc...

@AUTOMATIC1111
Copy link
Owner

#15205 implements callback order via extension configuration files so I'm closing this.

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.

3 participants