-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Support for finetuning Hunyuan Video-1.5 model #80
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
Conversation
Signed-off-by: Pranav Prashant Thombre <pthombre@nvidia.com>
Signed-off-by: Pranav Prashant Thombre <pthombre@nvidia.com>
Signed-off-by: Pranav Prashant Thombre <pthombre@nvidia.com>
Signed-off-by: Pranav Prashant Thombre <pthombre@nvidia.com>
Signed-off-by: Pranav Prashant Thombre <pthombre@nvidia.com>
56f6f0f to
aa475ee
Compare
|
|
||
|
|
||
| @dataclass | ||
| class FlowMatchingContext: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this context limited to just flow matching? if i'm not missing something EDM pipeline requires the same set of attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not specifically. But since I've implemented only flow matching thought to call it this.
|
|
||
| return sigma, timesteps, sampling_method | ||
|
|
||
| def _sample_from_distribution(self, batch_size: int, device: torch.device) -> torch.Tensor: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shall we separate this out of flow matching pipeline? we can have time step scheduler classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I felt like the sampling process is an intrinsic part of the flow matching logic so included it here. But I would also be okay to move it out if the goal is to develop a more scheduler class.
|
/ok to test aa475ee |
huvunvidia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment. Thanks for the PR.
|
|
||
|
|
||
| @dataclass | ||
| class FlowMatchingOutput: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class doesn't seem to be used anywhere?
Signed-off-by: Pranav Prashant Thombre <pthombre@nvidia.com>
Signed-off-by: Pranav Prashant Thombre <pthombre@nvidia.com>
|
/ok to test 33ea6fc |
|
@NVIDIA-NeMo/automation PTAL thanks |
pyproject.toml
Outdated
| dependencies = [ | ||
| "accelerate", | ||
| "diffusers==0.35.1", | ||
| "diffusers==0.36.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the motivation behind this strict pin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mainly being extra cautious due to the early stage of the dfm repo. I recommend to open another issue to ensure best-practices are being followed with regards to dependency management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a fair call out. We need the diffusers package version >=0.36.0. I will update the toml file accordingly.
Signed-off-by: Pranav Prashant Thombre <pthombre@nvidia.com>
|
/ok to test 375bba6 |
|
/ok to test ede8fca |
No description provided.