refactor: rename noise => predicted_noise

[brycedrennan]

noise => predicted_noise
denoised_x => renoised_x (denoised seems inaccurate since it does have noise added)

[limiteinductive]

noise => predicted_noise denoised_x => renoised_x (denoised seems inaccurate since it does have noise added)

A propos renoised_x, I get that since it is not fully denoised but only slightly denoised, denoised_x is a bit ambiguous; maybe intermediary_denoised_x or partially_denoised_x is more accurate, but they feel a bit too much for me.

For me, renoising suggests that we're stationary in the level of noise

[limiteinductive]

noise => predicted_noise denoised_x => renoised_x (denoised seems inaccurate since it does have noise added)

and noise => predicted_noise is ok for me

[brycedrennan]

Yeah that makes sense. I think just calling it next_x would be less misleading. Of all the ideas so far though, I do like slightly_denoised_x best- it conveys exactly what it is, even if its a long name. It is at least not used outside the function.

I was suspicious that labeling it "denoised" contributed to the bug of it being returned in the final step. I'm also suspicious that a similar bug exists in the other schedulers but I haven't investigated enough yet.

These changes were in the context of a not-finished pull request where I make the following change in ddim (and was looking to see if similar changes applied in the other schedulers):

        if step == self.num_inference_steps - 1:
            noise_factor = 0

to

        if step == self.num_inference_steps - 1:
            return predicted_x

[catwell]

As far as I understand it the additional noise term is not really a "bug", at least not in refiners, it's a discrepancy with the original implementation of SD (CompVis) which we used as a reference. See the documentation of the set_alpha_to_one option in Diffusers, for instance. It did not come from the naming.

Regarding other schedulers, the ability to use a 1st order update for the last step in DPM(++) solver that I added last week is somehow similar. It will remove the same kind of noise which is visible especially if you use SDXL with few timesteps. For now we decided against making it the default.

[brycedrennan]

I have reduced the scope of this pull request to just noise => predicted_noise

[brycedrennan]

@catwell thanks for the context - that makes sense

[brycedrennan]

We've discussed a few changes here and I'm happy to do all or none of them. Let me know what is desired to get this closed out.

[deltheil]

We've discussed a few changes here and I'm happy to do all or none of them. Let me know what is desired to get this closed out.

How about this:

1. Let's keep the noise => predicted_noise renaming (makes things more readable)
2. Re: schedulers/euler.py => let's keep the implementation as-is, except the noise => predicted_noise and alt_noise => noise renamings. Rationale: we're going to revamp it (completely) at some point

@brycedrennan does that work for you? Many thanks!

[brycedrennan]

sounds good - done

[deltheil]

Thanks!