Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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 because in DownBlockMotion, we make use of
resnet_groupstoo andtemporal_norm_num_groupsdoes not seem to be present there. I think this is correct but not too sure. Not doing this causes multiple errors with the group norm in_channels being indivisible by num_groups. There is no control overtemporal_norm_num_groupsparameter because it is not present inget_up_block(). Everywhere else that passes norm_num_groups into TemporalTransformerModel does what is present here.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.
Let's not touch the default values here please especially when we're not too sure. Will defer to @DN6 to verify.
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.
Yeah this change should be safe. I think it was a mistake here passing in
temporal_norm_num_groupsUh oh!
There was an error while loading. Please reload this page.
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.
awesome! What are your thoughts on the first failing test after these changes? If I make any changes there, multiple other tests are most likely going to break. We are trying to load hf-internal-testing/tiny-stable-diffusion-pipe weights into a different model definition due to the changes in unet causing weight mismatch...
Uh oh!
There was an error while loading. Please reload this page.
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.
That's okay. We can make a change to that model repo or change the test specifically for animatediff.