Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multipliers feature seems to have broken Modifiers feature #24

Open
kjerk opened this issue Jun 20, 2022 · 2 comments
Open

Multipliers feature seems to have broken Modifiers feature #24

kjerk opened this issue Jun 20, 2022 · 2 comments

Comments

@kjerk
Copy link

kjerk commented Jun 20, 2022

Replication:

python disco.py --config_file=./examples/configs/artstudy2.yml --width_height="[256, 256]"

Problem 1:

The first problem that shows up immediately is that split_prompts errors because it's always expecting a multi valued array for prompts for some reason, but this can be fixed by doing something that doesn't require the post-init assignment like

def split_prompts(prompts, max_frames=None):
    prompt_series = pd.Series([v for k, v in prompts.items()])
    prompt_series = prompt_series.ffill().bfill()
    return prompt_series

Problem 2 (main one):

It seems as though after the multipliers feature went in, using modifiers on its own does not work.
multargs is being passed as the first argument to processModifiers():

disco-diffusion-1/dd.py

Lines 2218 to 2219 in da528b4

multargs = processMultipliers(args=pargs)
jobs = processModifiers(multargs)

However in the implementation, processModifiers has two args and only the second is iterated over, so this method is always only returning the 1 job.

disco-diffusion-1/dd.py

Lines 2123 to 2125 in da528b4

def processModifiers(mods=[], args=[]):
for p in range(len(args)):
# Deep copy

I tried the simple workaround of changing this to

multargs = processMultipliers(args=pargs)
jobs = processModifiers(args=multargs)

but this wasn't enough, and the problem must run deeper than the more correct argument placement. The only workaround I got working locally was to comment out processMultipliers() and revert processModifiers() to the version from 1f5af86

@entmike
Copy link
Owner

entmike commented Jul 5, 2022

Thanks I am going to look at this, this week. I've been on vacation and putting out other fires but this is not ignored.

@kjerk
Copy link
Author

kjerk commented Jul 5, 2022

In the meanwhile I've been using the reverted version and the multipliers feature has been extremely useful for doing parameter studies, thanks for your work on 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

No branches or pull requests

2 participants