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

Remove some predict call duplication #41

Merged
merged 4 commits into from
Nov 1, 2024
Merged

Conversation

yorickvP
Copy link
Contributor

@yorickvP yorickvP commented Nov 1, 2024

  • fp8_predict already comes up with a seed, make base_predict also do this so they're compatible
  • fix fp8_predict/base_predict usage in lora predictors (it didn't take any aspect_ratio argument)
  • refactor away some code duplication

@yorickvP yorickvP changed the title Decouple resolution and seed preprocessing Remove some predict call duplication Nov 1, 2024
Copy link
Collaborator

@daanelson daanelson left a comment

Choose a reason for hiding this comment

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

@yorickvP this is great! thanks for catching it. going to handle the comment I mentioned about seeding and then I'll merge.

@@ -324,6 +318,10 @@ def base_predict(
torch_device = torch.device("cuda")
init_image = None

if not seed:
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm. fp8 predict does come up with a seed, but there's value in logging the seed s.t. the user can see it for subsequent predictions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah jk it's there isn't it. fair enough, this'll work.

@daanelson daanelson merged commit 5af9df0 into main Nov 1, 2024
1 check passed
@daanelson daanelson deleted the yorickvp/unify-seeds branch November 1, 2024 23:17
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.

2 participants