-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Modify FlowMatch Scale Noise #8678
Conversation
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
@@ -852,7 +852,7 @@ def __call__( | |||
# 4. Prepare timesteps | |||
timesteps, num_inference_steps = retrieve_timesteps(self.scheduler, num_inference_steps, device, timesteps) | |||
timesteps, num_inference_steps = self.get_timesteps(num_inference_steps, strength, device) | |||
latent_timestep = timesteps[:1].repeat(batch_size * num_inference_steps) | |||
latent_timestep = timesteps[:1].repeat(batch_size * num_images_per_prompt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😱😱😱😱
thanks for the fix!
I copied the almost the same logic than the |
There was a problem hiding this 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! left one more comment and let's run slow test for sd3 img2img to make sure it works
@@ -129,7 +129,31 @@ def scale_noise( | |||
if self.step_index is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can also remove this here
Let's remove it and run the example for img2img to see if the results match from main
and this branch
the test look good! since you got same results from main and branch |
I don't think the fix in the pipeline will affect the output, but I can't tell the difference, and the test slice is the same so I think we are fine |
ohh wait a second .... let's not merge just yet |
where is the difference in the image? |
you can open each one in a different tab and circle between them. The pear is a little bigger to the left and the stem is also a little higher |
pear.mp4 |
if self.step_index is None: | ||
self._init_step_index(timestep) | ||
# Make sure sigmas and timesteps have the same device and dtype as original_samples | ||
sigmas = self.sigmas.to(device=sample.device, dtype=sample.dtype) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the difference comes from casting simgas dtype
do you think the quality is worse when we cast the sigmas to fp16? (this branch)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, IMO is not worse, probably if I didn't use an example with a single fruit against a black background, I wouldn’t have even noticed the difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok let's merge then :)
* initial fix * apply suggestion * delete step_index line
What does this PR do?
This PR modifies
FlowMatchEulerDiscreteScheduler
so it can work with differential diffusion, also fixesStableDiffusion3Img2ImgPipeline
to work with the new change.I just made it analogous the other schedulers, but I found that there's a tiny change in the image to image generation, so I don't know if there's something wrong with what I did.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@yiyixuxu