Skip to content

Control lora trainer hunyuan #373

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 60 commits into
base: main
Choose a base branch
from
Draft

Control lora trainer hunyuan #373

wants to merge 60 commits into from

Conversation

@neph1
Copy link
Contributor

@neph1 neph1 commented Apr 12, 2025 •
edited
Loading

Still have some cleaning up to do, but this pr is back (I might migrate this and force push, or if it's eventually squashed).
It does things and produced something. I will do a longer run tomorrow, but I have no idea how to inference. Is there an example script somewhere i can modify? Found it

a-r-r-o-w and others added 30 commits March 9, 2025 23:48
@a-r-r-o-w
update
dc85bbf
@a-r-r-o-w
Merge branch 'main' into feature/control-lora-trainer
755fee8
@a-r-r-o-w
update
8812036
@a-r-r-o-w
update
e39d255
@a-r-r-o-w
update
18bd9ce
@a-r-r-o-w
update
3ef07fc
@a-r-r-o-w
update
ea07973
@neph1
add valid names to dataset docs
9f3d2cb
@a-r-r-o-w
update
2af75b1
@a-r-r-o-w
update
28b86c8
@a-r-r-o-w
update
84ffbd3
@a-r-r-o-w
update
1684ee5
@a-r-r-o-w
update
483e891
@a-r-r-o-w
update
657fb74
@a-r-r-o-w
update
cd859b3
@a-r-r-o-w
update
8cea261
@a-r-r-o-w
Merge branch 'main' into feature/control-lora-trainer
90d6d38
@a-r-r-o-w
fix
45bbf22
@a-r-r-o-w
update
825976d
@a-r-r-o-w
Merge branch 'main' into feature/control-lora-trainer
053757d
@a-r-r-o-w
update
eaafeab
@a-r-r-o-w
Merge branch 'feature/control-lora-trainer' of https://github.com/a-r…
9144f28 
…-r-o-w/finetrainers into feature/control-lora-trainer
@a-r-r-o-w
update
3745ae5
@neph1
Merge branch 'main' of github.com:neph1/finetrainers
d7ba5e1
@a-r-r-o-w
update
7245b5a
@a-r-r-o-w
update
c1c600f
@a-r-r-o-w
Merge branch 'main' into feature/control-lora-trainer
e1ef448
@a-r-r-o-w
update
8587874
@a-r-r-o-w
Merge branch 'main' into feature/control-lora-trainer
495e2b1
@a-r-r-o-w
Merge branch 'feature/control-lora-trainer' into feature/control-lora…
322d610 
…-trainer-wan
neph1
neph1 commented Apr 12, 2025
View reviewed changes
finetrainers/models/hunyuan_video/base_specification.py
video = video.permute(0, 2, 1, 3, 4).contiguous() # [B, F, C, H, W] -> [B, C, F, H, W]


compute_posterior = False
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So far only made it work with compute_posterior false

a-r-r-o-w
a-r-r-o-w reviewed Apr 13, 2025
View reviewed changes
Copy link
Contributor

@a-r-r-o-w a-r-r-o-w left a comment

Choose a reason for hiding this comment

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

Hey @neph1, thanks for the PR and testing with some runs! I know there's some duplication of code at the moment, but I plan to address that in the future with something else. For now, let's keep the duplication

I'll try to help with launching some training runs too for verifying the PR once we have the changes looking similar to the Wan/CogView control implementations #338

@neph1
Copy link
Contributor Author

neph1 commented Apr 13, 2025

Oh, I did follow the new implementation, just kept it on the same branch. At least I believe it follows what is in main now.
Not sure it works, though. I ran 1000 steps canny training on some simpsons dataset I found, think I managed to make the conversion script handle x_embedder, but it's only noise. Sadly can't inference with plain diffusers due to VRAM.

Got a bit disheartened, but I'll get back to it.

@neph1 neph1 force-pushed the control-lora-trainer-hunyuan branch from 2a06b9c to 9be0e0c Compare April 13, 2025 17:36
a-r-r-o-w
a-r-r-o-w reviewed Apr 18, 2025
View reviewed changes
finetrainers/models/hunyuan_video/base_specification.py Outdated
latents = moments.to(dtype=dtype)

return {self.output_names[0]: latents}
latents_mean = torch.tensor(vae.latent_channels)
Copy link
Contributor

Choose a reason for hiding this comment

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

@neph1 These changes seem incorrec to me and will cause worse generations. The previous implementation that did not perform this normalization was correct, I think.

Was this modified from Wan? If so, it's incorrect because they are different models and preprocess latents differently

@a-r-r-o-w
Copy link
Contributor

Also, I'm a bit more free now. I was working on a major upcoming feature for speeding up training and inference, and it's nearing completion. If you'd like me to take over the PR and make the relevant changes, do a long run for validating correctness, please do LMK

@neph1
Copy link
Contributor Author

neph1 commented Apr 18, 2025

By all means, if you have the time. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@a-r-r-o-w a-r-r-o-w a-r-r-o-w left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@neph1 @a-r-r-o-w