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

Generalize preview event #6777

Closed

Conversation

StAlKeR7779
Copy link
Contributor

Summary

Changes preview event the way, that it accepts image instead of latents.
This allow to use custom latents preview logic, for example - flux or use taesd for sd preview.

Related Issues / Discussions

#6739

QA Instructions

-

Merge Plan

-

Checklist

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

@brandonrising @RyanJDick

@github-actions github-actions bot added python PRs that change python files invocations PRs that change invocations backend PRs that change backend files services PRs that change app services labels Aug 24, 2024
@psychedelicious
Copy link
Collaborator

I have done something similar in #6715

I paused on working on it bc focusing on canvas

@StAlKeR7779
Copy link
Contributor Author

StAlKeR7779 commented Aug 24, 2024

I have done something similar in #6715

I paused on working on it bc focusing on canvas

Yeah, but here idea for now - unhardcode backend, without logical/data changes, so that my code still use old event, just it's construction logic moved to be able provide different latents->image part.

Copy link
Collaborator

@brandonrising brandonrising left a comment

Choose a reason for hiding this comment

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

lgtm

If schedulers will be rewriten in future there high chance that it field will be unused/removed.
Copy link
Collaborator

@RyanJDick RyanJDick left a comment

Choose a reason for hiding this comment

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

Just noticed a couple of minor typing errors.

I ran a quick smoke test. If someone has tested that context.util.sd_step_callback() still works, then this looks good to me.

@@ -265,7 +276,7 @@ def latents_from_embeddings(
seed: int,
timesteps: torch.Tensor,
init_timestep: torch.Tensor,
callback: Callable[[PipelineIntermediateState], None],
callback: Callable[[...], None],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Invalid type annotation. The correct syntax is:

Suggested change
callback: Callable[[...], None],
callback: Callable[..., None],

cls,
sample: torch.Tensor,
base_model: BaseModelType,
) -> Tuple[Image, int]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Type hint is wrong

Suggested change
) -> Tuple[Image, int]:
) -> Tuple[Image.Image, int]:

Copy link
Collaborator

@psychedelicious psychedelicious left a comment

Choose a reason for hiding this comment

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

See #6715 for a more generalized approach that would supersede the public API changes in this PR.

@psychedelicious
Copy link
Collaborator

superseded by #6908

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 services PRs that change app services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants