Skip to content

[Tests] clean up and refactor gradient checkpointing tests #9494

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

Merged
merged 11 commits into from
Oct 31, 2024

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