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

Fix some small things #18

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Fix some small things #18

wants to merge 17 commits into from

Conversation

tolgacangoz
Copy link
Contributor

@tolgacangoz tolgacangoz commented Oct 11, 2024

Proposes to fix #10

  1. This pull request updates the cc12m_256x256.yaml and cc12m_1024x1024.yaml files by adding a diffusion_config section because samplers are created with diffusion_config.sampler_config:

    self.sampler = samplers.Sampler(diffusion_config.sampler_config)

    self.sampler = samplers.NestedSampler(diffusion_config.sampler_config)

  2. Also, diffusion_config.no_use_residual is needed:

    if not self.diffusion_config.no_use_residual:

  3. None in the file is read by 'None' -as a string.

Proposes to fix #6

Fixes old module mimicry.

This pull request proposes a refactoring of the samplers.py file to ensure correct handling of scaling when the config.schedule_shifted flag is set to True. This change improves the behavior of the code, especially for higher resolutions.

Prompt = a blue jay stops on the top of a helmet of Japanese samurai, background with sakura tree
Guidance scale = 7.5
Thresholding = clip
Number of steps = 250

Before 1024x1024 After 1024x1024
before after

@MultiPath @Shuangfei @luke-carlson

@tolgacangoz tolgacangoz changed the title Update diffusion_config and fix issues with vision model download and scaling Fix all Oct 11, 2024
@tolgacangoz tolgacangoz changed the title Fix all Propose to fix all Oct 11, 2024
@tolgacangoz tolgacangoz marked this pull request as ready for review October 11, 2024 09:22
@tolgacangoz tolgacangoz changed the title Propose to fix all Proposes to fix them all Oct 11, 2024
@tolgacangoz tolgacangoz changed the title Proposes to fix them all One PR to fix them all, One PR to find them, One PR to bring them all, and in the darkness bind them Oct 11, 2024
@tolgacangoz tolgacangoz changed the title One PR to fix them all, One PR to find them, One PR to bring them all, and in the darkness bind them One PR to fix them all, One PR to find them, One PR to bring them all, and in the darkness bind them Oct 11, 2024
Copy link
Collaborator

@luke-carlson luke-carlson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me so far, PR is undergoing internal review now

configs/models/cc12m_256x256.yaml Show resolved Hide resolved
configs/models/cc12m_1024x1024.yaml Outdated Show resolved Hide resolved
@MultiPath
Copy link
Collaborator

#21

Thanks @tolgacangoz I think your PR fixed most of the issues. Please check the above my proposal (i think it did not fix the typos you mentioned)

ml_mdm/samplers.py Outdated Show resolved Hide resolved
@tolgacangoz tolgacangoz changed the title One PR to fix them all, One PR to find them, One PR to bring them all, and in the darkness bind them Fix some small things Oct 16, 2024
@tolgacangoz
Copy link
Contributor Author

This PR now fixes some small nits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants