KEMBAR78
[Tests] clean up and refactor gradient checkpointing tests by sayakpaul · Pull Request #9494 · huggingface/diffusers · GitHub
Skip to content

Conversation

sayakpaul
Copy link
Member

@sayakpaul sayakpaul commented Sep 23, 2024

What does this PR do?

Gradient checkpointing is an essential component of model training. We need to ensure it's implemented properly.

If we had them implemented properly we could have avoided the fix from #9489 beforehand. Another related issue: #9496.

Additionally, I took the liberty of properly skipping the related tests with "unittest.skip". This way, we know that tests are being actually skipped.

Comment on lines +341 to +343
@unittest.skip(
"Gradient checkpointing is supported but this test doesn't apply to this class because it's forward is a bit different from the rest."
)
Copy link
Member Author

Choose a reason for hiding this comment

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

Other forwards don't apply scaling and unscaling like it does. So, that makes this a little different.

@sayakpaul
Copy link
Member Author

@yiyixuxu WDYT?

@sayakpaul sayakpaul requested a review from yiyixuxu September 23, 2024 06:14
@sayakpaul sayakpaul requested a review from DN6 October 18, 2024 05:58
@sayakpaul
Copy link
Member Author

@DN6 could you give this a look?

@sayakpaul
Copy link
Member Author

@DN6 a gentle ping here.

@sayakpaul
Copy link
Member Author

Failing test is unrelated.

@sayakpaul sayakpaul merged commit 4adf6af into main Oct 31, 2024
13 of 14 checks passed
@sayakpaul sayakpaul deleted the gradient-ckpt-tests branch October 31, 2024 12:54
a-r-r-o-w pushed a commit that referenced this pull request Nov 1, 2024
* check.

* fixes

* fixes

* updates

* fixes

* fixes
sayakpaul added a commit that referenced this pull request Dec 23, 2024
* check.

* fixes

* fixes

* updates

* fixes

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants