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

Split up latent.py (code reorganization, no functional changes) #6491

Merged
merged 10 commits into from
Jun 7, 2024

Conversation

RyanJDick
Copy link
Collaborator

Summary

I've started working towards a better tiled upscaling implementation. It is going to require some refactoring of DenoiseLatentsInvocation. As a first step, this PR splits up all of the invocations in latent.py into their own files. That file had become a bit of a dumping ground - it should be a bit more manageable to work with now.

This PR just re-organizes the code. There should be no functional changes.

QA Instructions

I've done some light smoke testing. I'll do some more before merging. The main risk is that I missed a broken import, or some other copy-paste error.

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): N/A

@github-actions github-actions bot added python PRs that change python files invocations PRs that change invocations labels Jun 6, 2024
@RyanJDick RyanJDick mentioned this pull request Jun 6, 2024
5 tasks
Copy link
Collaborator

@blessedcoolant blessedcoolant left a comment

Choose a reason for hiding this comment

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

Maybe just rename latents.py to denoise_latents.py then? Rest of it seems good. This file needed to be decoupled.

@RyanJDick RyanJDick requested a review from lstein as a code owner June 6, 2024 17:46
@github-actions github-actions bot added Root python-deps PRs that change python dependencies labels Jun 6, 2024
@RyanJDick
Copy link
Collaborator Author

Maybe just rename latents.py to denoise_latents.py then? Rest of it seems good. This file needed to be decoupled.

Good point. Renamed in 68ab579

@blessedcoolant blessedcoolant self-requested a review June 6, 2024 17:49
Copy link
Collaborator

@lstein lstein left a comment

Choose a reason for hiding this comment

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

Looks great. I'd simply suggest changing latent.py to denoise_latents.py in order to be consistent with the naming scheme.

[edit] I see blessedcoolant had the same suggestion; did that commit just go through as I was reviewing?

Copy link
Collaborator

@lstein lstein left a comment

Choose a reason for hiding this comment

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

I see the change to latents.py now. Approved.

@RyanJDick RyanJDick merged commit 0dbec3a into main Jun 7, 2024
14 checks passed
@RyanJDick RyanJDick deleted the ryan/split-up-latent-py branch June 7, 2024 16:01
@skunkworxdark
Copy link
Contributor

The import path for SchedulerOutput in the invocation_api needs to be changed
it should be changed from

from invokeai.app.invocations.denoise_latents import SchedulerOutput

to

from invokeai.app.invocations.scheduler import SchedulerOutput

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invocations PRs that change invocations python PRs that change python files python-deps PRs that change python dependencies Root
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants