Skip to content

Conversation

@patrickvonplaten
Copy link
Contributor

No description provided.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Aug 31, 2022

The documentation is not available anymore as the PR was closed or merged.


raise ValueError(f"`self.tensor_format`: {self.tensor_format} is not valid.")

def set_seed(self, seed):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should not work with set_seed(...) here as it goes against the generator design. IMO passing generators around is the correct thing to do here because:

  • We cannot really pass seeds around and globally set manual_seed(...) every time:
    • It's not a great design to retrieve the current seed from a generator and then pass this -> better to pass generator directly
    • Flax passes PNRG keys around that can be split -> we cannot split PyTorch seeds in the same way and we cannot pass a seed into a torch.randn(...) function -> so let's go for the generator here

@patrickvonplaten patrickvonplaten changed the title [Refactor] Remove set_seed and class attributes [Refactor] Remove set_seed Aug 31, 2022
Copy link
Member

@pcuenca pcuenca 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!

for _ in range(self.scheduler.correct_steps):
model_output = self.unet(sample, sigma_t)["sample"]
sample = self.scheduler.step_correct(model_output, sample)["prev_sample"]
sample = self.scheduler.step_correct(model_output, sample, generator=generator)["prev_sample"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

let's pass the generator here


# 2. compute previous image: x_t -> t_t-1
image = self.scheduler.step(model_output, t, image)["prev_sample"]
image = self.scheduler.step(model_output, t, image, generator=generator)["prev_sample"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DDPM scheduler is also stochastic

Copy link
Member

@anton-l anton-l left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! Let's see how the tests behave now

Copy link
Contributor

@patil-suraj patil-suraj left a comment

Choose a reason for hiding this comment

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

LGTM!

@patrickvonplaten patrickvonplaten merged commit f3937bc into main Aug 31, 2022
@patrickvonplaten patrickvonplaten deleted the refactors branch August 31, 2022 17:29
natolambert pushed a commit that referenced this pull request Sep 7, 2022
* [Refactor] Remove set_seed and class attributes

* apply anton's suggestiosn

* fix

* Apply suggestions from code review

Co-authored-by: Pedro Cuenca <[email protected]>

* up

* update

* make style

* Apply suggestions from code review

Co-authored-by: Anton Lozhkov <[email protected]>

* make fix-copies

* make style

* make style and new copies

Co-authored-by: Pedro Cuenca <[email protected]>
Co-authored-by: Anton Lozhkov <[email protected]>
PhaneeshB added a commit to nod-ai/diffusers that referenced this pull request Mar 1, 2023
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
* [Refactor] Remove set_seed and class attributes

* apply anton's suggestiosn

* fix

* Apply suggestions from code review

Co-authored-by: Pedro Cuenca <[email protected]>

* up

* update

* make style

* Apply suggestions from code review

Co-authored-by: Anton Lozhkov <[email protected]>

* make fix-copies

* make style

* make style and new copies

Co-authored-by: Pedro Cuenca <[email protected]>
Co-authored-by: Anton Lozhkov <[email protected]>
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

Successfully merging this pull request may close these issues.

6 participants