Skip to content

Conversation

psychedelicious
Copy link
Collaborator

@psychedelicious psychedelicious commented Apr 11, 2024

Summary

Two fixes here. Both contribute noticeably to the results when inpainting with refiner.

fix(nodes): do not set seed on output latents from denoise latents

LatentsField objects have an optional seed field. This should only be populated when the latents are noise, generated from a seed.

DenoiseLatentsInvocation needs a seed value for scheduler initialization. It's used in a few places, and there is some logic for determining the seed to use with a series of fallbacks:

  • Use the seed from the noise (a LatentsField object)
  • Use the seed from the latents (a LatentsField object - normally it won't have a seed)
  • Use 0 as a final fallback

In DenoisLatentsInvocation, we set the seed in the LatentsOutput, even though the output latents are not noise.

This is normally fine, but when we use refiner, we re-use the those same latents for the refiner denoise. This causes that characteristic same-seed-fried look on the refiner pass.

Simple fix - do not set the field in the output latents.

Example outputs

These include the doubly-noised latents fix from below. The output is much worse without that.

Initial image:
image

Before After
image image
image image
image image

fix(nodes): doubly-noised latents

When using refiner with a mask (i.e. inpainting), we don't have noise provided as an input to the node.

This situation uniquely hits a code path that wasn't reviewed when gradient denoising was implemented.

That code path does two things wrong:

  • It lerp'd the input latents. This was fixed in 5a1f4cb.
  • It added noise to the latents an extra time. This is fixed in this change.

We don't need to add noise in latents_from_embeddings because we do it just a lines later in AddsMaskGuidance.

  • Remove the extraneous call to add_noise
  • Make seed a required arg. We never call the function without seed anyways. If we refactor this in the future, it will be clearer that we need to look at how seed is handled.
  • Move the call to create the noise to a deeper conditional, just before we call AddsMaskGuidance. The created noise tensor is now only used in that function, no need to create it every time.

Note: Whether or not having both noise and latents as inputs on the node is correct is a separate conversation. This change just fixes the issue with the current setup.

Example outputs

Initial image:
image

Before After
image image
image image
image image

Related Issues / Discussions

https://discord.com/channels/1020123559063990373/1149513625321603162/1227798332567715953

QA Instructions

Do some inpainting w/ refiner (and non-refiner). Should work great.

Merge Plan

n/a

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable) n/a
  • Documentation added / updated (if applicable) (added comments)

`LatentsField` objects have an optional `seed` field. This should only be populated when the latents are noise, generated from a seed.

`DenoiseLatentsInvocation` needs a seed value for scheduler initialization. It's used in a few places, and there is some logic for determining the seed to use with a series of fallbacks:
- Use the seed from the noise (a `LatentsField` object)
- Use the seed from the latents (a `LatentsField` object - normally it won't have a seed)
- Use `0` as a final fallback

In `DenoisLatentsInvocation`, we set the seed in the `LatentsOutput`, even though the output latents are not noise.

This is normally fine, but when we use refiner, we re-use the those same latents for the refiner denoise. This causes that characteristic same-seed-fried look on the refiner pass.

Simple fix - do not set the field in the output latents.
@github-actions github-actions bot added python PRs that change python files invocations PRs that change invocations backend PRs that change backend files labels Apr 11, 2024
When using refiner with a mask (i.e. inpainting), we don't have noise provided as an input to the node.

This situation uniquely hits a code path that wasn't reviewed when gradient denoising was implemented.

That code path does two things wrong:
- It lerp'd the input latents. This was fixed in 5a1f4cb.
- It added noise to the latents an extra time. This is fixed in this change.

We don't need to add noise in `latents_from_embeddings` because we do it just a lines later in `AddsMaskGuidance`.

- Remove the extraneous call to `add_noise`
- Make `seed` a required arg. We never call the function without seed anyways. If we refactor this in the future, it will be clearer that we need to look at how seed is handled.
- Move the call to create the noise to a deeper conditional, just before we call `AddsMaskGuidance`. The created noise tensor is now only used in that function, no need to create it every time.

Note: Whether or not having both noise and latents as inputs on the node is correct is a separate conversation. This change just fixes the issue with the current setup.
@psychedelicious psychedelicious force-pushed the psyche/fix/nodes/refiner-inpainting branch from 365767d to 8c537bb Compare April 11, 2024 05:29
@hipsterusername hipsterusername merged commit 7bc77dd into main Apr 11, 2024
@hipsterusername hipsterusername deleted the psyche/fix/nodes/refiner-inpainting branch April 11, 2024 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend PRs that change backend files invocations PRs that change invocations python PRs that change python files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants